Fixed correct short URL being tracked when domain exists

This commit is contained in:
Alejandro Celaya 2020-02-01 13:03:48 +01:00
parent fd82de31c0
commit 1b2a0d674f
4 changed files with 6 additions and 17 deletions

View file

@ -46,7 +46,6 @@ abstract class AbstractTrackingAction implements MiddlewareInterface
public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface
{ {
$identifier = ShortUrlIdentifier::fromRedirectRequest($request); $identifier = ShortUrlIdentifier::fromRedirectRequest($request);
$shortCode = $identifier->shortCode();
$query = $request->getQueryParams(); $query = $request->getQueryParams();
$disableTrackParam = $this->appOptions->getDisableTrackParam(); $disableTrackParam = $this->appOptions->getDisableTrackParam();
@ -55,7 +54,7 @@ abstract class AbstractTrackingAction implements MiddlewareInterface
// Track visit to this short code // Track visit to this short code
if ($disableTrackParam === null || ! array_key_exists($disableTrackParam, $query)) { if ($disableTrackParam === null || ! array_key_exists($disableTrackParam, $query)) {
$this->visitTracker->track($shortCode, Visitor::fromRequest($request)); $this->visitTracker->track($url, Visitor::fromRequest($request));
} }
return $this->createSuccessResp($this->buildUrlToRedirectTo($url, $query, $disableTrackParam)); return $this->createSuccessResp($this->buildUrlToRedirectTo($url, $query, $disableTrackParam));

View file

@ -31,13 +31,8 @@ class VisitsTracker implements VisitsTrackerInterface
/** /**
* Tracks a new visit to provided short code from provided visitor * Tracks a new visit to provided short code from provided visitor
*/ */
public function track(string $shortCode, Visitor $visitor): void public function track(ShortUrl $shortUrl, Visitor $visitor): void
{ {
/** @var ShortUrl $shortUrl */
$shortUrl = $this->em->getRepository(ShortUrl::class)->findOneBy([
'shortCode' => $shortCode,
]);
$visit = new Visit($shortUrl, $visitor); $visit = new Visit($shortUrl, $visitor);
$this->em->persist($visit); $this->em->persist($visit);

View file

@ -5,6 +5,7 @@ declare(strict_types=1);
namespace Shlinkio\Shlink\Core\Service; namespace Shlinkio\Shlink\Core\Service;
use Laminas\Paginator\Paginator; use Laminas\Paginator\Paginator;
use Shlinkio\Shlink\Core\Entity\ShortUrl;
use Shlinkio\Shlink\Core\Entity\Visit; use Shlinkio\Shlink\Core\Entity\Visit;
use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException;
use Shlinkio\Shlink\Core\Model\Visitor; use Shlinkio\Shlink\Core\Model\Visitor;
@ -15,7 +16,7 @@ interface VisitsTrackerInterface
/** /**
* Tracks a new visit to provided short code from provided visitor * Tracks a new visit to provided short code from provided visitor
*/ */
public function track(string $shortCode, Visitor $visitor): void; // FIXME public function track(ShortUrl $shortUrl, Visitor $visitor): void; // FIXME
/** /**
* Returns the visits on certain short code * Returns the visits on certain short code

View file

@ -39,14 +39,11 @@ class VisitsTrackerTest extends TestCase
public function trackPersistsVisit(): void public function trackPersistsVisit(): void
{ {
$shortCode = '123ABC'; $shortCode = '123ABC';
$repo = $this->prophesize(EntityRepository::class);
$repo->findOneBy(['shortCode' => $shortCode])->willReturn(new ShortUrl(''));
$this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal())->shouldBeCalledOnce();
$this->em->persist(Argument::that(fn (Visit $visit) => $visit->setId('1')))->shouldBeCalledOnce(); $this->em->persist(Argument::that(fn (Visit $visit) => $visit->setId('1')))->shouldBeCalledOnce();
$this->em->flush()->shouldBeCalledOnce(); $this->em->flush()->shouldBeCalledOnce();
$this->visitsTracker->track($shortCode, Visitor::emptyInstance()); $this->visitsTracker->track(new ShortUrl($shortCode), Visitor::emptyInstance());
$this->eventDispatcher->dispatch(Argument::type(ShortUrlVisited::class))->shouldHaveBeenCalled(); $this->eventDispatcher->dispatch(Argument::type(ShortUrlVisited::class))->shouldHaveBeenCalled();
} }
@ -55,10 +52,7 @@ class VisitsTrackerTest extends TestCase
public function trackedIpAddressGetsObfuscated(): void public function trackedIpAddressGetsObfuscated(): void
{ {
$shortCode = '123ABC'; $shortCode = '123ABC';
$repo = $this->prophesize(EntityRepository::class);
$repo->findOneBy(['shortCode' => $shortCode])->willReturn(new ShortUrl(''));
$this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal())->shouldBeCalledOnce();
$this->em->persist(Argument::any())->will(function ($args) { $this->em->persist(Argument::any())->will(function ($args) {
/** @var Visit $visit */ /** @var Visit $visit */
$visit = $args[0]; $visit = $args[0];
@ -68,7 +62,7 @@ class VisitsTrackerTest extends TestCase
})->shouldBeCalledOnce(); })->shouldBeCalledOnce();
$this->em->flush()->shouldBeCalledOnce(); $this->em->flush()->shouldBeCalledOnce();
$this->visitsTracker->track($shortCode, new Visitor('', '', '4.3.2.1')); $this->visitsTracker->track(new ShortUrl($shortCode), new Visitor('', '', '4.3.2.1'));
$this->eventDispatcher->dispatch(Argument::type(ShortUrlVisited::class))->shouldHaveBeenCalled(); $this->eventDispatcher->dispatch(Argument::type(ShortUrlVisited::class))->shouldHaveBeenCalled();
} }