Migrated TrackingOptions to immutable object

This commit is contained in:
Alejandro Celaya 2022-09-17 12:57:04 +02:00
parent 5f87bb13f8
commit fe4b2c4ae4
14 changed files with 77 additions and 162 deletions

View File

@ -43,8 +43,8 @@
"php-middleware/request-id": "^4.1",
"pugx/shortid-php": "^1.0",
"ramsey/uuid": "^4.3",
"shlinkio/shlink-common": "^5.0",
"shlinkio/shlink-config": "dev-main#33004e6 as 2.1",
"shlinkio/shlink-common": "dev-main#c9e6474 as 5.1",
"shlinkio/shlink-config": "dev-main#12fb295 as 2.1",
"shlinkio/shlink-event-dispatcher": "dev-main#48c0137 as 2.6",
"shlinkio/shlink-importer": "^4.0",
"shlinkio/shlink-installer": "dev-develop#a01bca9 as 8.2",

View File

@ -34,7 +34,9 @@ return (static function (): array {
'disable_ua_tracking' => (bool) EnvVars::DISABLE_UA_TRACKING->loadFromEnv(false),
// A list of IP addresses, patterns or CIDR blocks from which tracking is disabled by default
'disable_tracking_from' => $disableTrackingFrom === null ? [] : explode(',', $disableTrackingFrom),
'disable_tracking_from' => $disableTrackingFrom === null
? []
: array_map(trim(...), explode(',', $disableTrackingFrom)),
],
];

View File

@ -32,7 +32,7 @@ class GeolocationDbUpdater implements GeolocationDbUpdaterInterface
*/
public function checkDbUpdate(?callable $beforeDownload = null, ?callable $handleProgress = null): void
{
if ($this->trackingOptions->disableTracking() || $this->trackingOptions->disableIpTracking()) {
if ($this->trackingOptions->disableTracking || $this->trackingOptions->disableIpTracking) {
return;
}

View File

@ -29,7 +29,6 @@ class GeolocationDbUpdaterTest extends TestCase
private GeolocationDbUpdater $geolocationDbUpdater;
private ObjectProphecy $dbUpdater;
private ObjectProphecy $geoLiteDbReader;
private TrackingOptions $trackingOptions;
private ObjectProphecy $lock;
protected function setUp(): void
@ -38,19 +37,10 @@ class GeolocationDbUpdaterTest extends TestCase
$this->geoLiteDbReader = $this->prophesize(Reader::class);
$this->trackingOptions = new TrackingOptions();
$locker = $this->prophesize(Lock\LockFactory::class);
$this->lock = $this->prophesize(Lock\LockInterface::class);
$this->lock->acquire(true)->willReturn(true);
$this->lock->release()->will(function (): void {
});
$locker->createLock(Argument::type('string'))->willReturn($this->lock->reveal());
$this->geolocationDbUpdater = new GeolocationDbUpdater(
$this->dbUpdater->reveal(),
$this->geoLiteDbReader->reveal(),
$locker->reveal(),
$this->trackingOptions,
);
}
/** @test */
@ -64,7 +54,7 @@ class GeolocationDbUpdaterTest extends TestCase
$download = $this->dbUpdater->downloadFreshCopy(null)->willThrow($prev);
try {
$this->geolocationDbUpdater->checkDbUpdate($mustBeUpdated);
$this->geolocationDbUpdater()->checkDbUpdate($mustBeUpdated);
self::assertTrue(false); // If this is reached, the test will fail
} catch (Throwable $e) {
/** @var GeolocationDbUpdateFailedException $e */
@ -94,7 +84,7 @@ class GeolocationDbUpdaterTest extends TestCase
$download = $this->dbUpdater->downloadFreshCopy(null)->willThrow($prev);
try {
$this->geolocationDbUpdater->checkDbUpdate();
$this->geolocationDbUpdater()->checkDbUpdate();
self::assertTrue(false); // If this is reached, the test will fail
} catch (Throwable $e) {
/** @var GeolocationDbUpdateFailedException $e */
@ -127,7 +117,7 @@ class GeolocationDbUpdaterTest extends TestCase
$download = $this->dbUpdater->downloadFreshCopy(null)->will(function (): void {
});
$this->geolocationDbUpdater->checkDbUpdate();
$this->geolocationDbUpdater()->checkDbUpdate();
$fileExists->shouldHaveBeenCalledOnce();
$getMeta->shouldHaveBeenCalledOnce();
@ -160,7 +150,7 @@ class GeolocationDbUpdaterTest extends TestCase
$getMeta->shouldBeCalledOnce();
$download->shouldNotBeCalled();
$this->geolocationDbUpdater->checkDbUpdate();
$this->geolocationDbUpdater()->checkDbUpdate();
}
private function buildMetaWithBuildEpoch(string|int $buildEpoch): Metadata
@ -182,13 +172,9 @@ class GeolocationDbUpdaterTest extends TestCase
* @test
* @dataProvider provideTrackingOptions
*/
public function downloadDbIsSkippedIfTrackingIsDisabled(array $props): void
public function downloadDbIsSkippedIfTrackingIsDisabled(TrackingOptions $options): void
{
foreach ($props as $prop) {
$this->trackingOptions->{$prop} = true;
}
$this->geolocationDbUpdater->checkDbUpdate();
$this->geolocationDbUpdater($options)->checkDbUpdate();
$this->dbUpdater->databaseFileExists(Argument::cetera())->shouldNotHaveBeenCalled();
$this->geoLiteDbReader->metadata(Argument::cetera())->shouldNotHaveBeenCalled();
@ -196,8 +182,21 @@ class GeolocationDbUpdaterTest extends TestCase
public function provideTrackingOptions(): iterable
{
yield 'disableTracking' => [['disableTracking']];
yield 'disableIpTracking' => [['disableIpTracking']];
yield 'both' => [['disableTracking', 'disableIpTracking']];
yield 'disableTracking' => [new TrackingOptions(disableTracking: true)];
yield 'disableIpTracking' => [new TrackingOptions(disableIpTracking: true)];
yield 'both' => [new TrackingOptions(disableTracking: true, disableIpTracking: true)];
}
private function geolocationDbUpdater(?TrackingOptions $options = null): GeolocationDbUpdater
{
$locker = $this->prophesize(Lock\LockFactory::class);
$locker->createLock(Argument::type('string'))->willReturn($this->lock->reveal());
return new GeolocationDbUpdater(
$this->dbUpdater->reveal(),
$this->geoLiteDbReader->reveal(),
$locker->reveal(),
$options ?? new TrackingOptions(),
);
}
}

View File

@ -7,6 +7,7 @@ namespace Shlinkio\Shlink\Core;
use Laminas\ServiceManager\AbstractFactory\ConfigAbstractFactory;
use Laminas\ServiceManager\Factory\InvokableFactory;
use Psr\EventDispatcher\EventDispatcherInterface;
use Shlinkio\Shlink\Config\Factory\ValinorConfigFactory;
use Shlinkio\Shlink\Core\ErrorHandler;
use Shlinkio\Shlink\Core\Options\NotFoundRedirectOptions;
use Shlinkio\Shlink\Importer\ImportedLinksProcessorInterface;
@ -25,7 +26,7 @@ return [
Options\NotFoundRedirectOptions::class => ConfigAbstractFactory::class,
Options\RedirectOptions::class => ConfigAbstractFactory::class,
Options\UrlShortenerOptions::class => ConfigAbstractFactory::class,
Options\TrackingOptions::class => ConfigAbstractFactory::class,
Options\TrackingOptions::class => [ValinorConfigFactory::class, 'config.tracking'],
Options\QrCodeOptions::class => ConfigAbstractFactory::class,
Options\RabbitMqOptions::class => ConfigAbstractFactory::class,
Options\WebhookOptions::class => ConfigAbstractFactory::class,
@ -90,7 +91,6 @@ return [
Options\NotFoundRedirectOptions::class => ['config.not_found_redirects'],
Options\RedirectOptions::class => ['config.redirects'],
Options\UrlShortenerOptions::class => ['config.url_shortener'],
Options\TrackingOptions::class => ['config.tracking'],
Options\QrCodeOptions::class => ['config.qr_codes'],
Options\RabbitMqOptions::class => ['config.rabbitmq'],
Options\WebhookOptions::class => ['config.visits_webhooks'],

View File

@ -69,9 +69,9 @@ final class Visitor
public function normalizeForTrackingOptions(TrackingOptions $options): self
{
$instance = new self(
$options->disableUaTracking() ? '' : $this->userAgent,
$options->disableReferrerTracking() ? '' : $this->referer,
$options->disableIpTracking() ? null : $this->remoteAddress,
$options->disableUaTracking ? '' : $this->userAgent,
$options->disableReferrerTracking ? '' : $this->referer,
$options->disableIpTracking ? null : $this->remoteAddress,
$this->visitedUrl,
);

View File

@ -4,103 +4,21 @@ declare(strict_types=1);
namespace Shlinkio\Shlink\Core\Options;
use Laminas\Stdlib\AbstractOptions;
use function array_key_exists;
use function explode;
use function Functional\map;
use function is_array;
use function trim;
class TrackingOptions extends AbstractOptions
final class TrackingOptions
{
private bool $anonymizeRemoteAddr = true;
private bool $trackOrphanVisits = true;
private ?string $disableTrackParam = null;
private bool $disableTracking = false;
private bool $disableIpTracking = false;
private bool $disableReferrerTracking = false;
private bool $disableUaTracking = false;
private array $disableTrackingFrom = [];
public function anonymizeRemoteAddr(): bool
{
return $this->anonymizeRemoteAddr;
}
protected function setAnonymizeRemoteAddr(bool $anonymizeRemoteAddr): void
{
$this->anonymizeRemoteAddr = $anonymizeRemoteAddr;
}
public function trackOrphanVisits(): bool
{
return $this->trackOrphanVisits;
}
protected function setTrackOrphanVisits(bool $trackOrphanVisits): void
{
$this->trackOrphanVisits = $trackOrphanVisits;
}
public function getDisableTrackParam(): ?string
{
return $this->disableTrackParam;
}
public function queryHasDisableTrackParam(array $query): bool
{
return $this->disableTrackParam !== null && array_key_exists($this->disableTrackParam, $query);
}
protected function setDisableTrackParam(?string $disableTrackParam): void
{
$this->disableTrackParam = $disableTrackParam;
}
public function disableTracking(): bool
{
return $this->disableTracking;
}
protected function setDisableTracking(bool $disableTracking): void
{
$this->disableTracking = $disableTracking;
}
public function disableIpTracking(): bool
{
return $this->disableIpTracking;
}
protected function setDisableIpTracking(bool $disableIpTracking): void
{
$this->disableIpTracking = $disableIpTracking;
}
public function disableReferrerTracking(): bool
{
return $this->disableReferrerTracking;
}
protected function setDisableReferrerTracking(bool $disableReferrerTracking): void
{
$this->disableReferrerTracking = $disableReferrerTracking;
}
public function disableUaTracking(): bool
{
return $this->disableUaTracking;
}
protected function setDisableUaTracking(bool $disableUaTracking): void
{
$this->disableUaTracking = $disableUaTracking;
}
public function disableTrackingFrom(): array
{
return $this->disableTrackingFrom;
public function __construct(
public readonly bool $anonymizeRemoteAddr = true,
public readonly bool $trackOrphanVisits = true,
public readonly ?string $disableTrackParam = null,
public readonly bool $disableTracking = false,
public readonly bool $disableIpTracking = false,
public readonly bool $disableReferrerTracking = false,
public readonly bool $disableUaTracking = false,
/** @var string[] */
public readonly array $disableTrackingFrom = [],
) {
}
public function hasDisableTrackingFrom(): bool
@ -108,12 +26,8 @@ class TrackingOptions extends AbstractOptions
return ! empty($this->disableTrackingFrom);
}
protected function setDisableTrackingFrom(string|array|null $disableTrackingFrom): void
public function queryHasDisableTrackParam(array $query): bool
{
$this->disableTrackingFrom = match (true) {
is_array($disableTrackingFrom) => $disableTrackingFrom,
$disableTrackingFrom === null => [],
default => map(explode(',', $disableTrackingFrom), static fn (string $value) => trim($value)),
};
return $this->disableTrackParam !== null && array_key_exists($this->disableTrackParam, $query);
}
}

View File

@ -33,7 +33,7 @@ class ShortUrlRedirectionBuilder implements ShortUrlRedirectionBuilderInterface
{
$hardcodedQuery = Query::parse($uri->getQuery() ?? '');
$disableTrackParam = $this->trackingOptions->getDisableTrackParam();
$disableTrackParam = $this->trackingOptions->disableTrackParam;
if ($disableTrackParam !== null) {
unset($currentQuery[$disableTrackParam]);
}

View File

@ -83,7 +83,7 @@ class RequestTracker implements RequestTrackerInterface, RequestMethodInterface
}
$remoteAddrParts = explode('.', $remoteAddr);
$disableTrackingFrom = $this->trackingOptions->disableTrackingFrom();
$disableTrackingFrom = $this->trackingOptions->disableTrackingFrom;
return some($disableTrackingFrom, function (string $value) use ($ip, $remoteAddrParts): bool {
$range = str_contains($value, '*')

View File

@ -24,7 +24,7 @@ class VisitsTracker implements VisitsTrackerInterface
public function track(ShortUrl $shortUrl, Visitor $visitor): void
{
$this->trackVisit(
fn (Visitor $v) => Visit::forValidShortUrl($shortUrl, $v, $this->options->anonymizeRemoteAddr()),
fn (Visitor $v) => Visit::forValidShortUrl($shortUrl, $v, $this->options->anonymizeRemoteAddr),
$visitor,
);
}
@ -32,7 +32,7 @@ class VisitsTracker implements VisitsTrackerInterface
public function trackInvalidShortUrlVisit(Visitor $visitor): void
{
$this->trackOrphanVisit(
fn (Visitor $v) => Visit::forInvalidShortUrl($v, $this->options->anonymizeRemoteAddr()),
fn (Visitor $v) => Visit::forInvalidShortUrl($v, $this->options->anonymizeRemoteAddr),
$visitor,
);
}
@ -40,7 +40,7 @@ class VisitsTracker implements VisitsTrackerInterface
public function trackBaseUrlVisit(Visitor $visitor): void
{
$this->trackOrphanVisit(
fn (Visitor $v) => Visit::forBasePath($v, $this->options->anonymizeRemoteAddr()),
fn (Visitor $v) => Visit::forBasePath($v, $this->options->anonymizeRemoteAddr),
$visitor,
);
}
@ -48,14 +48,14 @@ class VisitsTracker implements VisitsTrackerInterface
public function trackRegularNotFoundVisit(Visitor $visitor): void
{
$this->trackOrphanVisit(
fn (Visitor $v) => Visit::forRegularNotFound($v, $this->options->anonymizeRemoteAddr()),
fn (Visitor $v) => Visit::forRegularNotFound($v, $this->options->anonymizeRemoteAddr),
$visitor,
);
}
private function trackOrphanVisit(callable $createVisit, Visitor $visitor): void
{
if (! $this->options->trackOrphanVisits()) {
if (! $this->options->trackOrphanVisits) {
return;
}
@ -64,7 +64,7 @@ class VisitsTracker implements VisitsTrackerInterface
private function trackVisit(callable $createVisit, Visitor $visitor): void
{
if ($this->options->disableTracking()) {
if ($this->options->disableTracking) {
return;
}

View File

@ -82,11 +82,11 @@ class VisitorTest extends TestCase
$this->generateRandomString(2000),
$this->generateRandomString(2000),
);
$normalizedVisitor = $visitor->normalizeForTrackingOptions(new TrackingOptions([
'disableIpTracking' => true,
'disableReferrerTracking' => true,
'disableUaTracking' => true,
]));
$normalizedVisitor = $visitor->normalizeForTrackingOptions(new TrackingOptions(
disableIpTracking: true,
disableReferrerTracking: true,
disableUaTracking: true,
));
self::assertNotSame($visitor, $normalizedVisitor);
self::assertEmpty($normalizedVisitor->userAgent);

View File

@ -16,7 +16,7 @@ class ShortUrlRedirectionBuilderTest extends TestCase
protected function setUp(): void
{
$trackingOptions = new TrackingOptions(['disable_track_param' => 'foobar']);
$trackingOptions = new TrackingOptions(disableTrackParam: 'foobar');
$this->redirectionBuilder = new ShortUrlRedirectionBuilder($trackingOptions);
}

View File

@ -38,10 +38,10 @@ class RequestTrackerTest extends TestCase
$this->requestTracker = new RequestTracker(
$this->visitsTracker->reveal(),
new TrackingOptions([
'disable_track_param' => 'foobar',
'disable_tracking_from' => ['80.90.100.110', '192.168.10.0/24', '1.2.*.*'],
]),
new TrackingOptions(
disableTrackParam: 'foobar',
disableTrackingFrom: ['80.90.100.110', '192.168.10.0/24', '1.2.*.*'],
),
);
$this->request = ServerRequestFactory::fromGlobals()->withAttribute(

View File

@ -24,16 +24,11 @@ class VisitsTrackerTest extends TestCase
private VisitsTracker $visitsTracker;
private ObjectProphecy $em;
private ObjectProphecy $eventDispatcher;
private TrackingOptions $options;
protected function setUp(): void
{
$this->em = $this->prophesize(EntityManager::class);
$this->eventDispatcher = $this->prophesize(EventDispatcherInterface::class);
$this->options = new TrackingOptions();
$this->visitsTracker = new VisitsTracker($this->em->reveal(), $this->eventDispatcher->reveal(), $this->options);
}
/**
@ -45,7 +40,7 @@ class VisitsTrackerTest extends TestCase
$persist = $this->em->persist(Argument::that(fn (Visit $visit) => $visit->setId('1')))->will(function (): void {
});
$this->visitsTracker->{$method}(...$args);
$this->visitsTracker()->{$method}(...$args);
$persist->shouldHaveBeenCalledOnce();
$this->em->flush()->shouldHaveBeenCalledOnce();
@ -58,9 +53,7 @@ class VisitsTrackerTest extends TestCase
*/
public function trackingIsSkippedCompletelyWhenDisabledFromOptions(string $method, array $args): void
{
$this->options->disableTracking = true;
$this->visitsTracker->{$method}(...$args);
$this->visitsTracker(new TrackingOptions(disableTracking: true))->{$method}(...$args);
$this->eventDispatcher->dispatch(Argument::cetera())->shouldNotHaveBeenCalled();
$this->em->persist(Argument::cetera())->shouldNotHaveBeenCalled();
@ -81,9 +74,7 @@ class VisitsTrackerTest extends TestCase
*/
public function orphanVisitsAreNotTrackedWhenDisabled(string $method): void
{
$this->options->trackOrphanVisits = false;
$this->visitsTracker->{$method}(Visitor::emptyInstance());
$this->visitsTracker(new TrackingOptions(trackOrphanVisits: false))->{$method}(Visitor::emptyInstance());
$this->eventDispatcher->dispatch(Argument::cetera())->shouldNotHaveBeenCalled();
$this->em->persist(Argument::cetera())->shouldNotHaveBeenCalled();
@ -96,4 +87,13 @@ class VisitsTrackerTest extends TestCase
yield 'trackBaseUrlVisit' => ['trackBaseUrlVisit'];
yield 'trackRegularNotFoundVisit' => ['trackRegularNotFoundVisit'];
}
private function visitsTracker(?TrackingOptions $options = null): VisitsTracker
{
return new VisitsTracker(
$this->em->reveal(),
$this->eventDispatcher->reveal(),
$options ?? new TrackingOptions(),
);
}
}