Added DTO used to pass filtering params to VisitsTracker

module/CLI/src/Command/ShortUrl/GetVisitsCommand.php

@@ -6,6 +6,7 @@ namespace Shlinkio\Shlink\CLI\Command\ShortUrl;
 use Cake\Chronos\Chronos;
 use Shlinkio\Shlink\Common\Util\DateRange;
 use Shlinkio\Shlink\Core\Entity\Visit;
+use Shlinkio\Shlink\Core\Model\VisitsParams;
 use Shlinkio\Shlink\Core\Service\VisitsTrackerInterface;
 use Symfony\Component\Console\Command\Command;
 use Symfony\Component\Console\Input\InputArgument;
@@ -72,7 +73,7 @@ class GetVisitsCommand extends Command
         $startDate = $this->getDateOption($input, 'startDate');
         $endDate = $this->getDateOption($input, 'endDate');
 
-        $visits = $this->visitsTracker->info($shortCode, new DateRange($startDate, $endDate));
+        $visits = $this->visitsTracker->info($shortCode, new VisitsParams(new DateRange($startDate, $endDate)));
         $rows = array_map(function (Visit $visit) {
             $rowData = $visit->jsonSerialize();
             $rowData['country'] = $visit->getVisitLocation()->getCountryName();

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

@@ -13,6 +13,7 @@ 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\Model\VisitsParams;
 use Shlinkio\Shlink\Core\Service\VisitsTrackerInterface;
 use Symfony\Component\Console\Application;
 use Symfony\Component\Console\Tester\CommandTester;
@@ -40,8 +41,8 @@ class GetVisitsCommandTest extends TestCase
     public function noDateFlagsTriesToListWithoutDateRange()
     {
         $shortCode = 'abc123';
-        $this->visitsTracker->info($shortCode, new DateRange(null, null))->willReturn([])
-                                                                         ->shouldBeCalledOnce();
+        $this->visitsTracker->info($shortCode, new VisitsParams(new DateRange(null, null)))->willReturn([])
+                                                                                           ->shouldBeCalledOnce();
 
         $this->commandTester->execute([
             'command' => 'shortcode:visits',
@@ -57,7 +58,10 @@ class GetVisitsCommandTest extends TestCase
         $shortCode = 'abc123';
         $startDate = '2016-01-01';
         $endDate = '2016-02-01';
-        $this->visitsTracker->info($shortCode, new DateRange(Chronos::parse($startDate), Chronos::parse($endDate)))
+        $this->visitsTracker->info(
+            $shortCode,
+            new VisitsParams(new DateRange(Chronos::parse($startDate), Chronos::parse($endDate)))
+        )
             ->willReturn([])
             ->shouldBeCalledOnce();
 

module/Core/src/Model/VisitsParams.php

@@ -0,0 +1,43 @@
+<?php
+declare(strict_types=1);
+
+namespace Shlinkio\Shlink\Core\Model;
+
+use Shlinkio\Shlink\Common\Util\DateRange;
+
+final class VisitsParams
+{
+    /** @var null|DateRange */
+    private $dateRange;
+    /** @var int */
+    private $page = 1;
+    /** @var null|int */
+    private $itemsPerPage;
+
+    public function __construct(?DateRange $dateRange = null, int $page = 1, ?int $itemsPerPage = null)
+    {
+        $this->dateRange = $dateRange ?? new DateRange();
+        $this->page = $page;
+        $this->itemsPerPage = $itemsPerPage;
+    }
+
+    public function getDateRange(): DateRange
+    {
+        return $this->dateRange;
+    }
+
+    public function getPage(): int
+    {
+        return $this->page;
+    }
+
+    public function getItemsPerPage(): ?int
+    {
+        return $this->itemsPerPage;
+    }
+
+    public function hasItemsPerPage(): bool
+    {
+        return $this->itemsPerPage !== null;
+    }
+}

module/Core/src/Service/VisitsTracker.php

@@ -4,11 +4,11 @@ declare(strict_types=1);
 namespace Shlinkio\Shlink\Core\Service;
 
 use Doctrine\ORM;
-use Shlinkio\Shlink\Common\Util\DateRange;
 use Shlinkio\Shlink\Core\Entity\ShortUrl;
 use Shlinkio\Shlink\Core\Entity\Visit;
 use Shlinkio\Shlink\Core\Exception\InvalidArgumentException;
 use Shlinkio\Shlink\Core\Model\Visitor;
+use Shlinkio\Shlink\Core\Model\VisitsParams;
 use Shlinkio\Shlink\Core\Repository\VisitRepository;
 use function sprintf;
 
@@ -43,12 +43,10 @@ class VisitsTracker implements VisitsTrackerInterface
     /**
      * Returns the visits on certain short code
      *
-     * @param string $shortCode
-     * @param DateRange $dateRange
      * @return Visit[]
      * @throws InvalidArgumentException
      */
-    public function info(string $shortCode, DateRange $dateRange = null): array
+    public function info(string $shortCode, VisitsParams $params): array
     {
         /** @var ShortUrl|null $shortUrl */
         $shortUrl = $this->em->getRepository(ShortUrl::class)->findOneBy([
@@ -60,6 +58,6 @@ class VisitsTracker implements VisitsTrackerInterface
 
         /** @var VisitRepository $repo */
         $repo = $this->em->getRepository(Visit::class);
-        return $repo->findVisitsByShortUrl($shortUrl, $dateRange);
+        return $repo->findVisitsByShortUrl($shortUrl, $params->getDateRange());
     }
 }

module/Core/src/Service/VisitsTrackerInterface.php

@@ -3,10 +3,10 @@ declare(strict_types=1);
 
 namespace Shlinkio\Shlink\Core\Service;
 
-use Shlinkio\Shlink\Common\Util\DateRange;
 use Shlinkio\Shlink\Core\Entity\Visit;
 use Shlinkio\Shlink\Core\Exception\InvalidArgumentException;
 use Shlinkio\Shlink\Core\Model\Visitor;
+use Shlinkio\Shlink\Core\Model\VisitsParams;
 
 interface VisitsTrackerInterface
 {
@@ -18,10 +18,8 @@ interface VisitsTrackerInterface
     /**
      * Returns the visits on certain short code
      *
-     * @param string $shortCode
-     * @param DateRange $dateRange
      * @return Visit[]
      * @throws InvalidArgumentException
      */
-    public function info(string $shortCode, DateRange $dateRange = null): array;
+    public function info(string $shortCode, VisitsParams $params): array;
 }

module/Core/test/Service/VisitsTrackerTest.php

@@ -8,9 +8,11 @@ use Doctrine\ORM\EntityRepository;
 use PHPUnit\Framework\TestCase;
 use Prophecy\Argument;
 use Prophecy\Prophecy\ObjectProphecy;
+use Shlinkio\Shlink\Common\Util\DateRange;
 use Shlinkio\Shlink\Core\Entity\ShortUrl;
 use Shlinkio\Shlink\Core\Entity\Visit;
 use Shlinkio\Shlink\Core\Model\Visitor;
+use Shlinkio\Shlink\Core\Model\VisitsParams;
 use Shlinkio\Shlink\Core\Repository\VisitRepository;
 use Shlinkio\Shlink\Core\Service\VisitsTracker;
 
@@ -80,9 +82,9 @@ class VisitsTrackerTest extends TestCase
             new Visit(new ShortUrl(''), Visitor::emptyInstance()),
         ];
         $repo2 = $this->prophesize(VisitRepository::class);
-        $repo2->findVisitsByShortUrl($shortUrl, null)->willReturn($list);
+        $repo2->findVisitsByShortUrl($shortUrl, Argument::type(DateRange::class))->willReturn($list);
         $this->em->getRepository(Visit::class)->willReturn($repo2->reveal())->shouldBeCalledOnce();
 
-        $this->assertEquals($list, $this->visitsTracker->info($shortCode));
+        $this->assertEquals($list, $this->visitsTracker->info($shortCode, new VisitsParams()));
     }
 }

module/Rest/src/Action/Visit/GetVisitsAction.php

@@ -4,12 +4,12 @@ declare(strict_types=1);
 namespace Shlinkio\Shlink\Rest\Action\Visit;
 
 use Cake\Chronos\Chronos;
-use Exception;
 use Psr\Http\Message\ResponseInterface as Response;
 use Psr\Http\Message\ServerRequestInterface as Request;
 use Psr\Log\LoggerInterface;
 use Shlinkio\Shlink\Common\Exception\InvalidArgumentException;
 use Shlinkio\Shlink\Common\Util\DateRange;
+use Shlinkio\Shlink\Core\Model\VisitsParams;
 use Shlinkio\Shlink\Core\Service\VisitsTrackerInterface;
 use Shlinkio\Shlink\Rest\Action\AbstractRestAction;
 use Shlinkio\Shlink\Rest\Util\RestUtils;
@@ -42,7 +42,7 @@ class GetVisitsAction extends AbstractRestAction
         $endDate = $this->getDateQueryParam($request, 'endDate');
 
         try {
-            $visits = $this->visitsTracker->info($shortCode, new DateRange($startDate, $endDate));
+            $visits = $this->visitsTracker->info($shortCode, new VisitsParams(new DateRange($startDate, $endDate)));
 
             return new JsonResponse([
                 'visits' => [
@@ -55,12 +55,6 @@ class GetVisitsAction extends AbstractRestAction
                 'error' => RestUtils::getRestErrorCodeFromException($e),
                 'message' => sprintf('Provided short code %s does not exist', $shortCode),
             ], self::STATUS_NOT_FOUND);
-        } catch (Exception $e) {
-            $this->logger->error('Unexpected error while parsing short code {e}', ['e' => $e]);
-            return new JsonResponse([
-                'error' => RestUtils::UNKNOWN_ERROR,
-                'message' => 'Unexpected error occurred',
-            ], self::STATUS_INTERNAL_SERVER_ERROR);
         }
     }
 

module/Rest/test/Action/Visit/GetVisitsActionTest.php

@@ -4,12 +4,12 @@ declare(strict_types=1);
 namespace ShlinkioTest\Shlink\Rest\Action\Visit;
 
 use Cake\Chronos\Chronos;
-use Exception;
 use PHPUnit\Framework\TestCase;
 use Prophecy\Argument;
 use Prophecy\Prophecy\ObjectProphecy;
 use Shlinkio\Shlink\Common\Exception\InvalidArgumentException;
 use Shlinkio\Shlink\Common\Util\DateRange;
+use Shlinkio\Shlink\Core\Model\VisitsParams;
 use Shlinkio\Shlink\Core\Service\VisitsTracker;
 use Shlinkio\Shlink\Rest\Action\Visit\GetVisitsAction;
 use Zend\Diactoros\ServerRequestFactory;
@@ -33,7 +33,7 @@ class GetVisitsActionTest extends TestCase
     public function providingCorrectShortCodeReturnsVisits()
     {
         $shortCode = 'abc123';
-        $this->visitsTracker->info($shortCode, Argument::type(DateRange::class))->willReturn([])
+        $this->visitsTracker->info($shortCode, Argument::type(VisitsParams::class))->willReturn([])
                                                                                 ->shouldBeCalledOnce();
 
         $response = $this->action->handle(ServerRequestFactory::fromGlobals()->withAttribute('shortCode', $shortCode));
@@ -46,7 +46,7 @@ class GetVisitsActionTest extends TestCase
     public function providingInvalidShortCodeReturnsError()
     {
         $shortCode = 'abc123';
-        $this->visitsTracker->info($shortCode, Argument::type(DateRange::class))->willThrow(
+        $this->visitsTracker->info($shortCode, Argument::type(VisitsParams::class))->willThrow(
             InvalidArgumentException::class
         )->shouldBeCalledOnce();
 
@@ -54,27 +54,15 @@ class GetVisitsActionTest extends TestCase
         $this->assertEquals(404, $response->getStatusCode());
     }
 
-    /**
-     * @test
-     */
-    public function unexpectedExceptionWillReturnError()
-    {
-        $shortCode = 'abc123';
-        $this->visitsTracker->info($shortCode, Argument::type(DateRange::class))->willThrow(
-            Exception::class
-        )->shouldBeCalledOnce();
-
-        $response = $this->action->handle(ServerRequestFactory::fromGlobals()->withAttribute('shortCode', $shortCode));
-        $this->assertEquals(500, $response->getStatusCode());
-    }
-
     /**
      * @test
      */
     public function datesAreReadFromQuery()
     {
         $shortCode = 'abc123';
-        $this->visitsTracker->info($shortCode, new DateRange(null, Chronos::parse('2016-01-01 00:00:00')))
+        $this->visitsTracker->info($shortCode, new VisitsParams(
+            new DateRange(null, Chronos::parse('2016-01-01 00:00:00'))
+        ))
             ->willReturn([])
             ->shouldBeCalledOnce();