Created named constructors for Visit entity and added tracking of the visited URL

This commit is contained in:
Alejandro Celaya 2021-02-07 21:31:12 +01:00
parent f5666c9451
commit 12b07bb0ac
19 changed files with 101 additions and 60 deletions

View File

@ -103,7 +103,7 @@ class GetVisitsCommandTest extends TestCase
$shortCode = 'abc123';
$this->visitsTracker->info(new ShortUrlIdentifier($shortCode), Argument::any())->willReturn(
new Paginator(new ArrayAdapter([
(new Visit(ShortUrl::createEmpty(), new Visitor('bar', 'foo', '')))->locate(
Visit::forValidShortUrl(ShortUrl::createEmpty(), new Visitor('bar', 'foo', '', ''))->locate(
new VisitLocation(new Location('', 'Spain', '', '', 0, 0, '')),
),
])),

View File

@ -77,7 +77,7 @@ class LocateVisitsCommandTest extends TestCase
bool $expectWarningPrint,
array $args
): void {
$visit = new Visit(ShortUrl::createEmpty(), new Visitor('', '', '1.2.3.4'));
$visit = Visit::forValidShortUrl(ShortUrl::createEmpty(), new Visitor('', '', '1.2.3.4', ''));
$location = new VisitLocation(Location::emptyInstance());
$mockMethodBehavior = $this->invokeHelperMethods($visit, $location);
@ -121,7 +121,7 @@ class LocateVisitsCommandTest extends TestCase
*/
public function localhostAndEmptyAddressesAreIgnored(?string $address, string $message): void
{
$visit = new Visit(ShortUrl::createEmpty(), new Visitor('', '', $address));
$visit = Visit::forValidShortUrl(ShortUrl::createEmpty(), new Visitor('', '', $address, ''));
$location = new VisitLocation(Location::emptyInstance());
$locateVisits = $this->visitService->locateUnlocatedVisits(Argument::cetera())->will(
@ -154,7 +154,7 @@ class LocateVisitsCommandTest extends TestCase
/** @test */
public function errorWhileLocatingIpIsDisplayed(): void
{
$visit = new Visit(ShortUrl::createEmpty(), new Visitor('', '', '1.2.3.4'));
$visit = Visit::forValidShortUrl(ShortUrl::createEmpty(), new Visitor('', '', '1.2.3.4', ''));
$location = new VisitLocation(Location::emptyInstance());
$locateVisits = $this->visitService->locateUnlocatedVisits(Argument::cetera())->will(

View File

@ -21,21 +21,27 @@ class Visit extends AbstractEntity implements JsonSerializable
private string $referer;
private Chronos $date;
private ?string $remoteAddr = null;
private ?string $visitedUrl = null;
private ?string $remoteAddr;
private ?string $visitedUrl;
private string $userAgent;
private string $type;
private ?ShortUrl $shortUrl;
private ?VisitLocation $visitLocation = null;
public function __construct(ShortUrl $shortUrl, Visitor $visitor, bool $anonymize = true, ?Chronos $date = null)
{
public function __construct(
?ShortUrl $shortUrl,
Visitor $visitor,
bool $anonymize = true,
?Chronos $date = null,
string $type = self::TYPE_VALID_SHORT_URL
) {
$this->shortUrl = $shortUrl;
$this->date = $date ?? Chronos::now();
$this->userAgent = $visitor->getUserAgent();
$this->referer = $visitor->getReferer();
$this->remoteAddr = $this->processAddress($anonymize, $visitor->getRemoteAddress());
$this->type = self::TYPE_VALID_SHORT_URL;
$this->visitedUrl = $visitor->getVisitedUrl();
$this->type = $type;
}
private function processAddress(bool $anonymize, ?string $address): ?string
@ -52,6 +58,26 @@ class Visit extends AbstractEntity implements JsonSerializable
}
}
public static function forValidShortUrl(ShortUrl $shortUrl, Visitor $visitor, bool $anonymize = true): self
{
return new self($shortUrl, $visitor, $anonymize);
}
public static function forBasePath(Visitor $visitor, bool $anonymize = true): self
{
return new self(null, $visitor, $anonymize, null, self::TYPE_BASE_URL);
}
public static function forInvalidShortUrl(Visitor $visitor, bool $anonymize = true): self
{
return new self(null, $visitor, $anonymize, null, self::TYPE_INVALID_SHORT_URL);
}
public static function forRegularNotFound(Visitor $visitor, bool $anonymize = true): self
{
return new self(null, $visitor, $anonymize, null, self::TYPE_REGULAR_404);
}
public function getRemoteAddr(): ?string
{
return $this->remoteAddr;

View File

@ -18,12 +18,14 @@ final class Visitor
private string $userAgent;
private string $referer;
private string $visitedUrl;
private ?string $remoteAddress;
public function __construct(string $userAgent, string $referer, ?string $remoteAddress)
public function __construct(string $userAgent, string $referer, ?string $remoteAddress, string $visitedUrl)
{
$this->userAgent = $this->cropToLength($userAgent, self::USER_AGENT_MAX_LENGTH);
$this->referer = $this->cropToLength($referer, self::REFERER_MAX_LENGTH);
$this->visitedUrl = $this->cropToLength($visitedUrl, self::VISITED_URL_MAX_LENGTH);
$this->remoteAddress = $this->cropToLength($remoteAddress, self::REMOTE_ADDRESS_MAX_LENGTH);
}
@ -38,12 +40,13 @@ final class Visitor
$request->getHeaderLine('User-Agent'),
$request->getHeaderLine('Referer'),
$request->getAttribute(IpAddressMiddlewareFactory::REQUEST_ATTR),
$request->getUri()->__toString(),
);
}
public static function emptyInstance(): self
{
return new self('', '', null);
return new self('', '', null, '');
}
public function getUserAgent(): string
@ -60,4 +63,9 @@ final class Visitor
{
return $this->remoteAddress;
}
public function getVisitedUrl(): string
{
return $this->visitedUrl;
}
}

View File

@ -41,7 +41,7 @@ class VisitsTracker implements VisitsTrackerInterface
public function track(ShortUrl $shortUrl, Visitor $visitor): void
{
$visit = new Visit($shortUrl, $visitor, $this->anonymizeRemoteAddr);
$visit = Visit::forValidShortUrl($shortUrl, $visitor, $this->anonymizeRemoteAddr);
$this->em->persist($visit);
$this->em->flush();

View File

@ -95,7 +95,7 @@ class ShortUrlRepositoryTest extends DatabaseTestCase
$this->getEntityManager()->persist($foo);
$bar = ShortUrl::withLongUrl('bar');
$visit = new Visit($bar, Visitor::emptyInstance());
$visit = Visit::forValidShortUrl($bar, Visitor::emptyInstance());
$this->getEntityManager()->persist($visit);
$bar->setVisits(new ArrayCollection([$visit]));
$this->getEntityManager()->persist($bar);

View File

@ -64,13 +64,13 @@ class TagRepositoryTest extends DatabaseTestCase
$shortUrl = ShortUrl::fromMeta($metaWithTags($firstUrlTags), $this->relationResolver);
$this->getEntityManager()->persist($shortUrl);
$this->getEntityManager()->persist(new Visit($shortUrl, Visitor::emptyInstance()));
$this->getEntityManager()->persist(new Visit($shortUrl, Visitor::emptyInstance()));
$this->getEntityManager()->persist(new Visit($shortUrl, Visitor::emptyInstance()));
$this->getEntityManager()->persist(Visit::forValidShortUrl($shortUrl, Visitor::emptyInstance()));
$this->getEntityManager()->persist(Visit::forValidShortUrl($shortUrl, Visitor::emptyInstance()));
$this->getEntityManager()->persist(Visit::forValidShortUrl($shortUrl, Visitor::emptyInstance()));
$shortUrl2 = ShortUrl::fromMeta($metaWithTags($secondUrlTags), $this->relationResolver);
$this->getEntityManager()->persist($shortUrl2);
$this->getEntityManager()->persist(new Visit($shortUrl2, Visitor::emptyInstance()));
$this->getEntityManager()->persist(Visit::forValidShortUrl($shortUrl2, Visitor::emptyInstance()));
$this->getEntityManager()->flush();
$result = $this->repo->findTagsWithInfo();

View File

@ -52,7 +52,7 @@ class VisitRepositoryTest extends DatabaseTestCase
};
for ($i = 0; $i < 6; $i++) {
$visit = new Visit($shortUrl, Visitor::emptyInstance());
$visit = Visit::forValidShortUrl($shortUrl, Visitor::emptyInstance());
if ($i >= 2) {
$location = new VisitLocation(Location::emptyInstance());

View File

@ -4,7 +4,6 @@ declare(strict_types=1);
namespace ShlinkioTest\Shlink\Core\Entity;
use Cake\Chronos\Chronos;
use PHPUnit\Framework\TestCase;
use Shlinkio\Shlink\Common\Util\IpAddress;
use Shlinkio\Shlink\Core\Entity\ShortUrl;
@ -13,35 +12,30 @@ use Shlinkio\Shlink\Core\Model\Visitor;
class VisitTest extends TestCase
{
/**
* @test
* @dataProvider provideDates
*/
public function isProperlyJsonSerialized(?Chronos $date): void
/** @test */
public function isProperlyJsonSerialized(): void
{
$visit = new Visit(ShortUrl::createEmpty(), new Visitor('Chrome', 'some site', '1.2.3.4'), true, $date);
$visit = Visit::forValidShortUrl(ShortUrl::createEmpty(), new Visitor('Chrome', 'some site', '1.2.3.4', ''));
self::assertEquals([
'referer' => 'some site',
'date' => ($date ?? $visit->getDate())->toAtomString(),
'date' => $visit->getDate()->toAtomString(),
'userAgent' => 'Chrome',
'visitLocation' => null,
], $visit->jsonSerialize());
}
public function provideDates(): iterable
{
yield 'null date' => [null];
yield 'not null date' => [Chronos::now()->subDays(10)];
}
/**
* @test
* @dataProvider provideAddresses
*/
public function addressIsAnonymizedWhenRequested(bool $anonymize, ?string $address, ?string $expectedAddress): void
{
$visit = new Visit(ShortUrl::createEmpty(), new Visitor('Chrome', 'some site', $address), $anonymize);
$visit = Visit::forValidShortUrl(
ShortUrl::createEmpty(),
new Visitor('Chrome', 'some site', $address, ''),
$anonymize,
);
self::assertEquals($expectedAddress, $visit->getRemoteAddr());
}

View File

@ -78,7 +78,7 @@ class LocateShortUrlVisitTest extends TestCase
{
$event = new ShortUrlVisited('123');
$findVisit = $this->em->find(Visit::class, '123')->willReturn(
new Visit(ShortUrl::createEmpty(), new Visitor('', '', '1.2.3.4')),
Visit::forValidShortUrl(ShortUrl::createEmpty(), new Visitor('', '', '1.2.3.4', '')),
);
$resolveLocation = $this->ipLocationResolver->resolveIpLocation(Argument::cetera())->willThrow(
WrongIpException::class,
@ -127,9 +127,9 @@ class LocateShortUrlVisitTest extends TestCase
{
$shortUrl = ShortUrl::createEmpty();
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))];
yield 'null IP' => [Visit::forValidShortUrl($shortUrl, new Visitor('', '', null, ''))];
yield 'empty IP' => [Visit::forValidShortUrl($shortUrl, new Visitor('', '', '', ''))];
yield 'localhost' => [Visit::forValidShortUrl($shortUrl, new Visitor('', '', IpAddress::LOCALHOST, ''))];
}
/**
@ -139,7 +139,7 @@ class LocateShortUrlVisitTest extends TestCase
public function locatableVisitsResolveToLocation(string $anonymizedIpAddress, ?string $originalIpAddress): void
{
$ipAddr = $originalIpAddress ?? $anonymizedIpAddress;
$visit = new Visit(ShortUrl::createEmpty(), new Visitor('', '', $ipAddr));
$visit = Visit::forValidShortUrl(ShortUrl::createEmpty(), new Visitor('', '', $ipAddr, ''));
$location = new Location('', '', '', '', 0.0, 0.0, '');
$event = new ShortUrlVisited('123', $originalIpAddress);
@ -171,7 +171,7 @@ class LocateShortUrlVisitTest extends TestCase
{
$e = GeolocationDbUpdateFailedException::withOlderDb();
$ipAddr = '1.2.3.0';
$visit = new Visit(ShortUrl::createEmpty(), new Visitor('', '', $ipAddr));
$visit = Visit::forValidShortUrl(ShortUrl::createEmpty(), new Visitor('', '', $ipAddr, ''));
$location = new Location('', '', '', '', 0.0, 0.0, '');
$event = new ShortUrlVisited('123');
@ -202,7 +202,7 @@ class LocateShortUrlVisitTest extends TestCase
{
$e = GeolocationDbUpdateFailedException::withoutOlderDb();
$ipAddr = '1.2.3.0';
$visit = new Visit(ShortUrl::createEmpty(), new Visitor('', '', $ipAddr));
$visit = Visit::forValidShortUrl(ShortUrl::createEmpty(), new Visitor('', '', $ipAddr, ''));
$location = new Location('', '', '', '', 0.0, 0.0, '');
$event = new ShortUrlVisited('123');

View File

@ -77,7 +77,7 @@ class NotifyVisitToMercureTest extends TestCase
public function notificationsAreSentWhenVisitIsFound(): void
{
$visitId = '123';
$visit = new Visit(ShortUrl::createEmpty(), Visitor::emptyInstance());
$visit = Visit::forValidShortUrl(ShortUrl::createEmpty(), Visitor::emptyInstance());
$update = new Update('', '');
$findVisit = $this->em->find(Visit::class, $visitId)->willReturn($visit);
@ -101,7 +101,7 @@ class NotifyVisitToMercureTest extends TestCase
public function debugIsLoggedWhenExceptionIsThrown(): void
{
$visitId = '123';
$visit = new Visit(ShortUrl::createEmpty(), Visitor::emptyInstance());
$visit = Visit::forValidShortUrl(ShortUrl::createEmpty(), Visitor::emptyInstance());
$update = new Update('', '');
$e = new RuntimeException('Error');

View File

@ -82,7 +82,7 @@ class NotifyVisitToWebHooksTest extends TestCase
$invalidWebhooks = ['invalid', 'baz'];
$find = $this->em->find(Visit::class, '1')->willReturn(
new Visit(ShortUrl::createEmpty(), Visitor::emptyInstance()),
Visit::forValidShortUrl(ShortUrl::createEmpty(), Visitor::emptyInstance()),
);
$requestAsync = $this->httpClient->requestAsync(
RequestMethodInterface::METHOD_POST,

View File

@ -35,7 +35,7 @@ class MercureUpdatesGeneratorTest extends TestCase
'longUrl' => '',
'title' => $title,
]));
$visit = new Visit($shortUrl, Visitor::emptyInstance());
$visit = Visit::forValidShortUrl($shortUrl, Visitor::emptyInstance());
$update = $this->generator->{$method}($visit);

View File

@ -31,7 +31,7 @@ class VisitorTest extends TestCase
public function provideParams(): iterable
{
yield 'all values are bigger' => [
[str_repeat('a', 1000), str_repeat('b', 2000), str_repeat('c', 500)],
[str_repeat('a', 1000), str_repeat('b', 2000), str_repeat('c', 500), ''],
[
'userAgent' => str_repeat('a', Visitor::USER_AGENT_MAX_LENGTH),
'referer' => str_repeat('b', Visitor::REFERER_MAX_LENGTH),
@ -39,7 +39,7 @@ class VisitorTest extends TestCase
],
];
yield 'some values are smaller' => [
[str_repeat('a', 10), str_repeat('b', 2000), null],
[str_repeat('a', 10), str_repeat('b', 2000), null, ''],
[
'userAgent' => str_repeat('a', 10),
'referer' => str_repeat('b', Visitor::REFERER_MAX_LENGTH),
@ -51,6 +51,7 @@ class VisitorTest extends TestCase
$userAgent = $this->generateRandomString(2000),
$referer = $this->generateRandomString(50),
null,
'',
],
[
'userAgent' => substr($userAgent, 0, Visitor::USER_AGENT_MAX_LENGTH),

View File

@ -34,7 +34,7 @@ class DeleteShortUrlServiceTest extends TestCase
public function setUp(): void
{
$shortUrl = ShortUrl::createEmpty()->setVisits(new ArrayCollection(
map(range(0, 10), fn () => new Visit(ShortUrl::createEmpty(), Visitor::emptyInstance())),
map(range(0, 10), fn () => Visit::forValidShortUrl(ShortUrl::createEmpty(), Visitor::emptyInstance())),
));
$this->shortCode = $shortUrl->getShortCode();

View File

@ -121,7 +121,7 @@ class ShortUrlResolverTest extends TestCase
$shortUrl = ShortUrl::fromMeta(ShortUrlMeta::fromRawData(['maxVisits' => 3, 'longUrl' => '']));
$shortUrl->setVisits(new ArrayCollection(map(
range(0, 4),
fn () => new Visit($shortUrl, Visitor::emptyInstance()),
fn () => Visit::forValidShortUrl($shortUrl, Visitor::emptyInstance()),
)));
return $shortUrl;
@ -140,7 +140,7 @@ class ShortUrlResolverTest extends TestCase
]));
$shortUrl->setVisits(new ArrayCollection(map(
range(0, 4),
fn () => new Visit($shortUrl, Visitor::emptyInstance()),
fn () => Visit::forValidShortUrl($shortUrl, Visitor::emptyInstance()),
)));
return $shortUrl;

View File

@ -73,7 +73,7 @@ class VisitsTrackerTest extends TestCase
$count = $repo->shortCodeIsInUse($shortCode, null, $spec)->willReturn(true);
$this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal())->shouldBeCalledOnce();
$list = map(range(0, 1), fn () => new Visit(ShortUrl::createEmpty(), Visitor::emptyInstance()));
$list = map(range(0, 1), fn () => Visit::forValidShortUrl(ShortUrl::createEmpty(), Visitor::emptyInstance()));
$repo2 = $this->prophesize(VisitRepository::class);
$repo2->findVisitsByShortCode($shortCode, null, Argument::type(DateRange::class), 1, 0, $spec)->willReturn(
$list,
@ -129,7 +129,7 @@ class VisitsTrackerTest extends TestCase
$getRepo = $this->em->getRepository(Tag::class)->willReturn($repo->reveal());
$spec = $apiKey === null ? null : $apiKey->spec();
$list = map(range(0, 1), fn () => new Visit(ShortUrl::createEmpty(), Visitor::emptyInstance()));
$list = map(range(0, 1), fn () => Visit::forValidShortUrl(ShortUrl::createEmpty(), Visitor::emptyInstance()));
$repo2 = $this->prophesize(VisitRepository::class);
$repo2->findVisitsByTag($tag, Argument::type(DateRange::class), 1, 0, $spec)->willReturn($list);
$repo2->countVisitsByTag($tag, Argument::type(DateRange::class), $spec)->willReturn(1);

View File

@ -57,7 +57,8 @@ class VisitLocatorTest extends TestCase
): void {
$unlocatedVisits = map(
range(1, 200),
fn (int $i) => new Visit(ShortUrl::withLongUrl(sprintf('short_code_%s', $i)), Visitor::emptyInstance()),
fn (int $i) =>
Visit::forValidShortUrl(ShortUrl::withLongUrl(sprintf('short_code_%s', $i)), Visitor::emptyInstance()),
);
$findVisits = $this->mockRepoMethod($expectedRepoMethodName)->willReturn($unlocatedVisits);
@ -107,7 +108,7 @@ class VisitLocatorTest extends TestCase
bool $isNonLocatableAddress
): void {
$unlocatedVisits = [
new Visit(ShortUrl::withLongUrl('foo'), Visitor::emptyInstance()),
Visit::forValidShortUrl(ShortUrl::withLongUrl('foo'), Visitor::emptyInstance()),
];
$findVisits = $this->mockRepoMethod($expectedRepoMethodName)->willReturn($unlocatedVisits);

View File

@ -22,19 +22,30 @@ class VisitsFixture extends AbstractFixture implements DependentFixtureInterface
{
/** @var ShortUrl $abcShortUrl */
$abcShortUrl = $this->getReference('abc123_short_url');
$manager->persist(new Visit($abcShortUrl, new Visitor('shlink-tests-agent', '', '44.55.66.77')));
$manager->persist(new Visit($abcShortUrl, new Visitor('shlink-tests-agent', 'https://google.com', '4.5.6.7')));
$manager->persist(new Visit($abcShortUrl, new Visitor('shlink-tests-agent', '', '1.2.3.4')));
$manager->persist(
Visit::forValidShortUrl($abcShortUrl, new Visitor('shlink-tests-agent', '', '44.55.66.77', '')),
);
$manager->persist(Visit::forValidShortUrl(
$abcShortUrl,
new Visitor('shlink-tests-agent', 'https://google.com', '4.5.6.7', ''),
));
$manager->persist(Visit::forValidShortUrl($abcShortUrl, new Visitor('shlink-tests-agent', '', '1.2.3.4', '')));
/** @var ShortUrl $defShortUrl */
$defShortUrl = $this->getReference('def456_short_url');
$manager->persist(new Visit($defShortUrl, new Visitor('shlink-tests-agent', '', '127.0.0.1')));
$manager->persist(new Visit($defShortUrl, new Visitor('shlink-tests-agent', 'https://app.shlink.io', '')));
$manager->persist(
Visit::forValidShortUrl($defShortUrl, new Visitor('shlink-tests-agent', '', '127.0.0.1', '')),
);
$manager->persist(
Visit::forValidShortUrl($defShortUrl, new Visitor('shlink-tests-agent', 'https://app.shlink.io', '', '')),
);
/** @var ShortUrl $ghiShortUrl */
$ghiShortUrl = $this->getReference('ghi789_short_url');
$manager->persist(new Visit($ghiShortUrl, new Visitor('shlink-tests-agent', '', '1.2.3.4')));
$manager->persist(new Visit($ghiShortUrl, new Visitor('shlink-tests-agent', 'https://app.shlink.io', '')));
$manager->persist(Visit::forValidShortUrl($ghiShortUrl, new Visitor('shlink-tests-agent', '', '1.2.3.4', '')));
$manager->persist(
Visit::forValidShortUrl($ghiShortUrl, new Visitor('shlink-tests-agent', 'https://app.shlink.io', '', '')),
);
$manager->flush();
}