From 8417498f08eea372b1e68f52444223b22127b5d0 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Wed, 27 Mar 2024 09:33:19 +0100 Subject: [PATCH] Fixes on static check and unit tests --- .../Command/ShortUrl/ListShortUrlsCommand.php | 10 ++++++++-- .../ShortUrl/ListShortUrlsCommandTest.php | 17 ++++++++++------- .../Model/ShortUrlWithVisitsSummary.php | 7 ++++++- .../ShortUrl/ShortUrlListServiceInterface.php | 4 ++-- .../Listener/ShortUrlVisitsCountTracker.php | 8 ++++---- 5 files changed, 30 insertions(+), 16 deletions(-) diff --git a/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php b/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php index b03e0312..4e3a7706 100644 --- a/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php +++ b/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php @@ -177,6 +177,9 @@ class ListShortUrlsCommand extends Command return ExitCode::EXIT_SUCCESS; } + /** + * @param array $columnsMap + */ private function renderPage( OutputInterface $output, array $columnsMap, @@ -186,8 +189,8 @@ class ListShortUrlsCommand extends Command $shortUrls = $this->shortUrlService->listShortUrls($params); $rows = map([...$shortUrls], function (ShortUrlWithVisitsSummary $shortUrl) use ($columnsMap) { - $rawShortUrl = $this->transformer->transform($shortUrl); - return map($columnsMap, fn (callable $call) => $call($rawShortUrl, $shortUrl)); + $serializedShortUrl = $this->transformer->transform($shortUrl); + return map($columnsMap, fn (callable $call) => $call($serializedShortUrl, $shortUrl->shortUrl)); }); ShlinkTable::default($output)->render( @@ -210,6 +213,9 @@ class ListShortUrlsCommand extends Command return $dir === null ? $field : sprintf('%s-%s', $field, $dir); } + /** + * @return array + */ private function resolveColumnsMap(InputInterface $input): array { $pickProp = static fn (string $prop): callable => static fn (array $shortUrl) => $shortUrl[$prop]; diff --git a/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php b/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php index 6859ec13..6016da1c 100644 --- a/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php +++ b/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php @@ -16,6 +16,7 @@ use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlStringifier; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlCreation; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlsParams; +use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlWithVisitsSummary; use Shlinkio\Shlink\Core\ShortUrl\Model\TagsMode; use Shlinkio\Shlink\Core\ShortUrl\ShortUrlListServiceInterface; use Shlinkio\Shlink\Core\ShortUrl\Transformer\ShortUrlDataTransformer; @@ -47,7 +48,7 @@ class ListShortUrlsCommandTest extends TestCase // The paginator will return more than one page $data = []; for ($i = 0; $i < 50; $i++) { - $data[] = ShortUrl::withLongUrl('https://url_' . $i); + $data[] = ShortUrlWithVisitsSummary::fromShortUrl(ShortUrl::withLongUrl('https://url_' . $i)); } $this->shortUrlService->expects($this->exactly(3))->method('listShortUrls')->withAnyParameters() @@ -69,7 +70,7 @@ class ListShortUrlsCommandTest extends TestCase // The paginator will return more than one page $data = []; for ($i = 0; $i < 30; $i++) { - $data[] = ShortUrl::withLongUrl('https://url_' . $i); + $data[] = ShortUrlWithVisitsSummary::fromShortUrl(ShortUrl::withLongUrl('https://url_' . $i)); } $this->shortUrlService->expects($this->once())->method('listShortUrls')->with( @@ -111,11 +112,13 @@ class ListShortUrlsCommandTest extends TestCase $this->shortUrlService->expects($this->once())->method('listShortUrls')->with( ShortUrlsParams::emptyInstance(), )->willReturn(new Paginator(new ArrayAdapter([ - ShortUrl::create(ShortUrlCreation::fromRawData([ - 'longUrl' => 'https://foo.com', - 'tags' => ['foo', 'bar', 'baz'], - 'apiKey' => $apiKey, - ])), + ShortUrlWithVisitsSummary::fromShortUrl( + ShortUrl::create(ShortUrlCreation::fromRawData([ + 'longUrl' => 'https://foo.com', + 'tags' => ['foo', 'bar', 'baz'], + 'apiKey' => $apiKey, + ])), + ), ]))); $this->commandTester->setInputs(['y']); diff --git a/module/Core/src/ShortUrl/Model/ShortUrlWithVisitsSummary.php b/module/Core/src/ShortUrl/Model/ShortUrlWithVisitsSummary.php index 79bdb526..50efaaee 100644 --- a/module/Core/src/ShortUrl/Model/ShortUrlWithVisitsSummary.php +++ b/module/Core/src/ShortUrl/Model/ShortUrlWithVisitsSummary.php @@ -9,7 +9,7 @@ use Shlinkio\Shlink\Core\Visit\Model\VisitsSummary; final readonly class ShortUrlWithVisitsSummary { - private function __construct(public ShortUrl $shortUrl, public VisitsSummary $visitsSummary) + private function __construct(public ShortUrl $shortUrl, private ?VisitsSummary $visitsSummary = null) { } @@ -24,6 +24,11 @@ final readonly class ShortUrlWithVisitsSummary )); } + public static function fromShortUrl(ShortUrl $shortUrl): self + { + return new self($shortUrl); + } + public function toArray(): array { return $this->shortUrl->toArray($this->visitsSummary); diff --git a/module/Core/src/ShortUrl/ShortUrlListServiceInterface.php b/module/Core/src/ShortUrl/ShortUrlListServiceInterface.php index ef7b31c2..ffbb1374 100644 --- a/module/Core/src/ShortUrl/ShortUrlListServiceInterface.php +++ b/module/Core/src/ShortUrl/ShortUrlListServiceInterface.php @@ -5,14 +5,14 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\ShortUrl; use Shlinkio\Shlink\Common\Paginator\Paginator; -use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlsParams; +use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlWithVisitsSummary; use Shlinkio\Shlink\Rest\Entity\ApiKey; interface ShortUrlListServiceInterface { /** - * @return ShortUrl[]|Paginator + * @return ShortUrlWithVisitsSummary[]|Paginator */ public function listShortUrls(ShortUrlsParams $params, ?ApiKey $apiKey = null): Paginator; } diff --git a/module/Core/src/Visit/Listener/ShortUrlVisitsCountTracker.php b/module/Core/src/Visit/Listener/ShortUrlVisitsCountTracker.php index f64974dd..25df0b83 100644 --- a/module/Core/src/Visit/Listener/ShortUrlVisitsCountTracker.php +++ b/module/Core/src/Visit/Listener/ShortUrlVisitsCountTracker.php @@ -48,14 +48,14 @@ final class ShortUrlVisitsCountTracker */ private function trackVisitCount(EntityManagerInterface $em, object $entity): void { - // This is not a non-orphan visit - if (!$entity instanceof Visit || $entity->shortUrl === null) { + // This is not a visit + if (!$entity instanceof Visit) { return; } $visit = $entity; - // The short URL is not persisted yet - $shortUrlId = $visit->shortUrl->getId(); + // The short URL is not persisted yet or this is an orphan visit + $shortUrlId = $visit->shortUrl?->getId(); if ($shortUrlId === null || $shortUrlId === '') { return; }