Improved public API in Visit entity, reducing anemic model

module/CLI/test/Command/ShortUrl/GetVisitsCommandTest.php

@@ -9,8 +9,10 @@ use Prophecy\Argument;
 use Prophecy\Prophecy\ObjectProphecy;
 use Shlinkio\Shlink\CLI\Command\ShortUrl\GetVisitsCommand;
 use Shlinkio\Shlink\Common\Util\DateRange;
+use Shlinkio\Shlink\Core\Entity\ShortUrl;
 use Shlinkio\Shlink\Core\Entity\Visit;
 use Shlinkio\Shlink\Core\Entity\VisitLocation;
+use Shlinkio\Shlink\Core\Model\Visitor;
 use Shlinkio\Shlink\Core\Service\VisitsTrackerInterface;
 use Symfony\Component\Console\Application;
 use Symfony\Component\Console\Tester\CommandTester;
@@ -79,9 +81,9 @@ class GetVisitsCommandTest extends TestCase
     {
         $shortCode = 'abc123';
         $this->visitsTracker->info($shortCode, Argument::any())->willReturn([
-            (new Visit())->setReferer('foo')
+            (new Visit(new ShortUrl(''), new Visitor('bar', 'foo', '')))->setVisitLocation(
-                         ->setVisitLocation(new VisitLocation(['country_name' => 'Spain']))
+                new VisitLocation(['country_name' => 'Spain'])
-                         ->setUserAgent('bar'),
+            ),
         ])->shouldBeCalledTimes(1);
 
         $this->commandTester->execute([

module/CLI/test/Command/Visit/ProcessVisitsCommandTest.php

@@ -8,7 +8,9 @@ use Prophecy\Argument;
 use Prophecy\Prophecy\ObjectProphecy;
 use Shlinkio\Shlink\CLI\Command\Visit\ProcessVisitsCommand;
 use Shlinkio\Shlink\Common\Service\IpApiLocationResolver;
+use Shlinkio\Shlink\Core\Entity\ShortUrl;
 use Shlinkio\Shlink\Core\Entity\Visit;
+use Shlinkio\Shlink\Core\Model\Visitor;
 use Shlinkio\Shlink\Core\Service\VisitService;
 use Symfony\Component\Console\Application;
 use Symfony\Component\Console\Output\OutputInterface;
@@ -54,10 +56,12 @@ class ProcessVisitsCommandTest extends TestCase
      */
     public function allReturnedVisitsIpsAreProcessed()
     {
+        $shortUrl = new ShortUrl('');
+
         $visits = [
-            (new Visit())->setRemoteAddr('1.2.3.4'),
+            new Visit($shortUrl, new Visitor('', '', '1.2.3.4')),
-            (new Visit())->setRemoteAddr('4.3.2.1'),
+            new Visit($shortUrl, new Visitor('', '', '4.3.2.1')),
-            (new Visit())->setRemoteAddr('12.34.56.78'),
+            new Visit($shortUrl, new Visitor('', '', '12.34.56.78')),
         ];
         $this->visitService->getUnlocatedVisits()->willReturn($visits)
                                                  ->shouldBeCalledTimes(1);
@@ -80,14 +84,16 @@ class ProcessVisitsCommandTest extends TestCase
      */
     public function localhostAndEmptyAddressIsIgnored()
     {
+        $shortUrl = new ShortUrl('');
+
         $visits = [
-            (new Visit())->setRemoteAddr('1.2.3.4'),
+            new Visit($shortUrl, new Visitor('', '', '1.2.3.4')),
-            (new Visit())->setRemoteAddr('4.3.2.1'),
+            new Visit($shortUrl, new Visitor('', '', '4.3.2.1')),
-            (new Visit())->setRemoteAddr('12.34.56.78'),
+            new Visit($shortUrl, new Visitor('', '', '12.34.56.78')),
-            (new Visit())->setRemoteAddr('127.0.0.1'),
+            new Visit($shortUrl, new Visitor('', '', '127.0.0.1')),
-            (new Visit())->setRemoteAddr('127.0.0.1'),
+            new Visit($shortUrl, new Visitor('', '', '127.0.0.1')),
-            (new Visit())->setRemoteAddr(''),
+            new Visit($shortUrl, new Visitor('', '', '')),
-            (new Visit())->setRemoteAddr(null),
+            new Visit($shortUrl, new Visitor('', '', null)),
         ];
         $this->visitService->getUnlocatedVisits()->willReturn($visits)
             ->shouldBeCalledTimes(1);
@@ -109,17 +115,19 @@ class ProcessVisitsCommandTest extends TestCase
      */
     public function sleepsEveryTimeTheApiLimitIsReached()
     {
+        $shortUrl = new ShortUrl('');
+
         $visits = [
-            (new Visit())->setRemoteAddr('1.2.3.4'),
+            new Visit($shortUrl, new Visitor('', '', '1.2.3.4')),
-            (new Visit())->setRemoteAddr('4.3.2.1'),
+            new Visit($shortUrl, new Visitor('', '', '4.3.2.1')),
-            (new Visit())->setRemoteAddr('12.34.56.78'),
+            new Visit($shortUrl, new Visitor('', '', '12.34.56.78')),
-            (new Visit())->setRemoteAddr('1.2.3.4'),
+            new Visit($shortUrl, new Visitor('', '', '1.2.3.4')),
-            (new Visit())->setRemoteAddr('4.3.2.1'),
+            new Visit($shortUrl, new Visitor('', '', '4.3.2.1')),
-            (new Visit())->setRemoteAddr('12.34.56.78'),
+            new Visit($shortUrl, new Visitor('', '', '12.34.56.78')),
-            (new Visit())->setRemoteAddr('1.2.3.4'),
+            new Visit($shortUrl, new Visitor('', '', '1.2.3.4')),
-            (new Visit())->setRemoteAddr('4.3.2.1'),
+            new Visit($shortUrl, new Visitor('', '', '4.3.2.1')),
-            (new Visit())->setRemoteAddr('12.34.56.78'),
+            new Visit($shortUrl, new Visitor('', '', '12.34.56.78')),
-            (new Visit())->setRemoteAddr('4.3.2.1'),
+            new Visit($shortUrl, new Visitor('', '', '4.3.2.1')),
         ];
         $apiLimit = 3;
 

module/Core/src/Entity/Visit.php

@@ -9,6 +9,7 @@ use JsonSerializable;
 use Shlinkio\Shlink\Common\Entity\AbstractEntity;
 use Shlinkio\Shlink\Common\Exception\WrongIpException;
 use Shlinkio\Shlink\Common\Util\IpAddress;
+use Shlinkio\Shlink\Core\Model\Visitor;
 use Shlinkio\Shlink\Core\Repository\VisitRepository;
 
 /**
@@ -54,58 +55,13 @@ class Visit extends AbstractEntity implements JsonSerializable
      */
     private $visitLocation;
 
-    public function __construct()
+    public function __construct(ShortUrl $shortUrl, Visitor $visitor, ?Chronos $date = null)
-    {
-        $this->date = Chronos::now();
-    }
-
-    public function getReferer(): string
-    {
-        return $this->referer;
-    }
-
-    public function setReferer(string $referer): self
-    {
-        $this->referer = $referer;
-        return $this;
-    }
-
-    public function getDate(): Chronos
-    {
-        return $this->date;
-    }
-
-    public function setDate(Chronos $date): self
-    {
-        $this->date = $date;
-        return $this;
-    }
-
-    public function getShortUrl(): ShortUrl
-    {
-        return $this->shortUrl;
-    }
-
-    public function setShortUrl(ShortUrl $shortUrl): self
     {
         $this->shortUrl = $shortUrl;
-        return $this;
+        $this->date = $date ?? Chronos::now();
-    }
+        $this->userAgent = $visitor->getUserAgent();
-
+        $this->referer = $visitor->getReferer();
-    public function getRemoteAddr(): ?string
+        $this->remoteAddr = $this->obfuscateAddress($visitor->getRemoteAddress());
-    {
-        return $this->remoteAddr;
-    }
-
-    public function setRemoteAddr(?string $remoteAddr): self
-    {
-        $this->remoteAddr = $this->obfuscateAddress($remoteAddr);
-        return $this;
-    }
-
-    public function hasRemoteAddr(): bool
-    {
-        return ! empty($this->remoteAddr);
     }
 
     private function obfuscateAddress(?string $address): ?string
@@ -122,15 +78,14 @@ class Visit extends AbstractEntity implements JsonSerializable
         }
     }
 
-    public function getUserAgent(): string
+    public function getRemoteAddr(): ?string
     {
-        return $this->userAgent;
+        return $this->remoteAddr;
     }
 
-    public function setUserAgent(string $userAgent): self
+    public function hasRemoteAddr(): bool
     {
-        $this->userAgent = $userAgent;
+        return ! empty($this->remoteAddr);
-        return $this;
     }
 
     public function getVisitLocation(): VisitLocation

module/Core/src/Service/VisitsTracker.php

@@ -34,11 +34,7 @@ class VisitsTracker implements VisitsTrackerInterface
             'shortCode' => $shortCode,
         ]);
 
-        $visit = new Visit();
+        $visit = new Visit($shortUrl, $visitor);
-        $visit->setShortUrl($shortUrl)
-              ->setUserAgent($visitor->getUserAgent())
-              ->setReferer($visitor->getReferer())
-              ->setRemoteAddr($visitor->getRemoteAddress());
 
         /** @var ORM\EntityManager $em */
         $em = $this->em;

module/Core/test-func/Repository/ShortUrlRepositoryTest.php

@@ -9,6 +9,7 @@ use Shlinkio\Shlink\Core\Entity\ShortUrl;
 use Shlinkio\Shlink\Core\Entity\Tag;
 use Shlinkio\Shlink\Core\Entity\Visit;
 use Shlinkio\Shlink\Core\Model\ShortUrlMeta;
+use Shlinkio\Shlink\Core\Model\Visitor;
 use Shlinkio\Shlink\Core\Repository\ShortUrlRepository;
 use ShlinkioTest\Shlink\Common\DbUnit\DatabaseTestCase;
 use function count;
@@ -44,13 +45,13 @@ class ShortUrlRepositoryTest extends DatabaseTestCase
         $bar->setShortCode('bar_very_long_text');
         $this->getEntityManager()->persist($bar);
 
+        $baz = new ShortUrl('baz', ShortUrlMeta::createFromRawData(['maxVisits' => 3]));
         $visits = [];
         for ($i = 0; $i < 3; $i++) {
-            $visit = new Visit();
+            $visit = new Visit($baz, Visitor::emptyInstance());
             $this->getEntityManager()->persist($visit);
             $visits[] = $visit;
         }
-        $baz = new ShortUrl('baz', ShortUrlMeta::createFromRawData(['maxVisits' => 3]));
         $baz->setShortCode('baz')
             ->setVisits(new ArrayCollection($visits));
         $this->getEntityManager()->persist($baz);

module/Core/test-func/Repository/VisitRepositoryTest.php

@@ -8,6 +8,7 @@ use Shlinkio\Shlink\Common\Util\DateRange;
 use Shlinkio\Shlink\Core\Entity\ShortUrl;
 use Shlinkio\Shlink\Core\Entity\Visit;
 use Shlinkio\Shlink\Core\Entity\VisitLocation;
+use Shlinkio\Shlink\Core\Model\Visitor;
 use Shlinkio\Shlink\Core\Repository\VisitRepository;
 use ShlinkioTest\Shlink\Common\DbUnit\DatabaseTestCase;
 use function sprintf;
@@ -35,8 +36,11 @@ class VisitRepositoryTest extends DatabaseTestCase
      */
     public function findUnlocatedVisitsReturnsProperVisits()
     {
+        $shortUrl = new ShortUrl('');
+        $this->getEntityManager()->persist($shortUrl);
+
         for ($i = 0; $i < 6; $i++) {
-            $visit = new Visit();
+            $visit = new Visit($shortUrl, Visitor::emptyInstance());
 
             if ($i % 2 === 0) {
                 $location = new VisitLocation([]);
@@ -60,10 +64,7 @@ class VisitRepositoryTest extends DatabaseTestCase
         $this->getEntityManager()->persist($shortUrl);
 
         for ($i = 0; $i < 6; $i++) {
-            $visit = new Visit();
+            $visit = new Visit($shortUrl, Visitor::emptyInstance(), Chronos::parse(sprintf('2016-01-0%s', $i + 1)));
-            $visit->setShortUrl($shortUrl)
-                  ->setDate(Chronos::parse(sprintf('2016-01-0%s', $i + 1)));
-
             $this->getEntityManager()->persist($visit);
         }
         $this->getEntityManager()->flush();

module/Core/test/Service/ShortUrl/DeleteShortUrlServiceTest.php

@@ -11,6 +11,7 @@ use Prophecy\Prophecy\ObjectProphecy;
 use Shlinkio\Shlink\Core\Entity\ShortUrl;
 use Shlinkio\Shlink\Core\Entity\Visit;
 use Shlinkio\Shlink\Core\Exception\DeleteShortUrlException;
+use Shlinkio\Shlink\Core\Model\Visitor;
 use Shlinkio\Shlink\Core\Options\DeleteShortUrlsOptions;
 use Shlinkio\Shlink\Core\Repository\ShortUrlRepositoryInterface;
 use Shlinkio\Shlink\Core\Service\ShortUrl\DeleteShortUrlService;
@@ -32,7 +33,7 @@ class DeleteShortUrlServiceTest extends TestCase
     {
         $shortUrl = (new ShortUrl(''))->setShortCode('abc123')
                                       ->setVisits(new ArrayCollection(array_map(function () {
-                                          return new Visit();
+                                          return new Visit(new ShortUrl(''), Visitor::emptyInstance());
                                       }, range(0, 10))));
 
         $this->em = $this->prophesize(EntityManagerInterface::class);

module/Core/test/Service/VisitServiceTest.php

@@ -6,7 +6,9 @@ namespace ShlinkioTest\Shlink\Core\Service;
 use Doctrine\ORM\EntityManager;
 use PHPUnit\Framework\TestCase;
 use Prophecy\Prophecy\ObjectProphecy;
+use Shlinkio\Shlink\Core\Entity\ShortUrl;
 use Shlinkio\Shlink\Core\Entity\Visit;
+use Shlinkio\Shlink\Core\Model\Visitor;
 use Shlinkio\Shlink\Core\Repository\VisitRepository;
 use Shlinkio\Shlink\Core\Service\VisitService;
 
@@ -32,7 +34,7 @@ class VisitServiceTest extends TestCase
      */
     public function saveVisitsPersistsProvidedVisit()
     {
-        $visit = new Visit();
+        $visit = new Visit(new ShortUrl(''), Visitor::emptyInstance());
         $this->em->persist($visit)->shouldBeCalledTimes(1);
         $this->em->flush()->shouldBeCalledTimes(1);
         $this->visitService->saveVisit($visit);

module/Core/test/Service/VisitsTrackerTest.php

@@ -80,8 +80,8 @@ class VisitsTrackerTest extends TestCase
         $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal())->shouldBeCalledTimes(1);
 
         $list = [
-            new Visit(),
+            new Visit(new ShortUrl(''), Visitor::emptyInstance()),
-            new Visit(),
+            new Visit(new ShortUrl(''), Visitor::emptyInstance()),
         ];
         $repo2 = $this->prophesize(VisitRepository::class);
         $repo2->findVisitsByShortUrl($shortUrl, null)->willReturn($list);