diff --git a/module/CLI/src/Command/ShortUrl/GetVisitsCommand.php b/module/CLI/src/Command/ShortUrl/GetVisitsCommand.php index 467a76fa..55c3a569 100644 --- a/module/CLI/src/Command/ShortUrl/GetVisitsCommand.php +++ b/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(); diff --git a/module/CLI/test/Command/ShortUrl/GetVisitsCommandTest.php b/module/CLI/test/Command/ShortUrl/GetVisitsCommandTest.php index 9c4947dd..33a7c662 100644 --- a/module/CLI/test/Command/ShortUrl/GetVisitsCommandTest.php +++ b/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(); diff --git a/module/Core/src/Model/VisitsParams.php b/module/Core/src/Model/VisitsParams.php new file mode 100644 index 00000000..734dbf57 --- /dev/null +++ b/module/Core/src/Model/VisitsParams.php @@ -0,0 +1,43 @@ +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; + } +} diff --git a/module/Core/src/Service/VisitsTracker.php b/module/Core/src/Service/VisitsTracker.php index 4bf8111a..d05735ed 100644 --- a/module/Core/src/Service/VisitsTracker.php +++ b/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()); } } diff --git a/module/Core/src/Service/VisitsTrackerInterface.php b/module/Core/src/Service/VisitsTrackerInterface.php index 79676864..e895dae3 100644 --- a/module/Core/src/Service/VisitsTrackerInterface.php +++ b/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; } diff --git a/module/Core/test/Service/VisitsTrackerTest.php b/module/Core/test/Service/VisitsTrackerTest.php index 11d71968..cc1d938c 100644 --- a/module/Core/test/Service/VisitsTrackerTest.php +++ b/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())); } } diff --git a/module/Rest/src/Action/Visit/GetVisitsAction.php b/module/Rest/src/Action/Visit/GetVisitsAction.php index bddc113d..aa128c8f 100644 --- a/module/Rest/src/Action/Visit/GetVisitsAction.php +++ b/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); } } diff --git a/module/Rest/test/Action/Visit/GetVisitsActionTest.php b/module/Rest/test/Action/Visit/GetVisitsActionTest.php index f9dd2e54..ed34e284 100644 --- a/module/Rest/test/Action/Visit/GetVisitsActionTest.php +++ b/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();