From d948543d5c6e3f0f4e1e3127617ae25e87f2b93e Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 24 Mar 2024 17:06:11 +0100 Subject: [PATCH] Wrap JSON serialization for any kind of visit in Visit entity itself --- module/Core/config/dependencies.config.php | 6 +- .../PublishingUpdatesGenerator.php | 10 +-- .../OrphanVisitDataTransformer.php | 19 ----- .../PublishingUpdatesGeneratorTest.php | 2 - module/Core/test/Visit/Entity/VisitTest.php | 61 +++++++++++++ .../OrphanVisitDataTransformerTest.php | 85 ------------------- module/Rest/config/dependencies.config.php | 5 +- .../src/Action/Visit/OrphanVisitsAction.php | 9 +- .../Action/Visit/OrphanVisitsActionTest.php | 9 +- 9 files changed, 71 insertions(+), 135 deletions(-) delete mode 100644 module/Core/src/Visit/Transformer/OrphanVisitDataTransformer.php delete mode 100644 module/Core/test/Visit/Transformer/OrphanVisitDataTransformerTest.php diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index ed64a30e..d75c6bb8 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -68,7 +68,6 @@ return [ Visit\Geolocation\VisitLocator::class => ConfigAbstractFactory::class, Visit\Geolocation\VisitToLocationHelper::class => ConfigAbstractFactory::class, Visit\VisitsStatsHelper::class => ConfigAbstractFactory::class, - Visit\Transformer\OrphanVisitDataTransformer::class => InvokableFactory::class, Visit\Repository\VisitLocationRepository::class => [ EntityRepositoryFactory::class, Visit\Entity\Visit::class, @@ -199,10 +198,7 @@ return [ ], ShortUrl\Middleware\TrimTrailingSlashMiddleware::class => [Options\UrlShortenerOptions::class], - EventDispatcher\PublishingUpdatesGenerator::class => [ - ShortUrl\Transformer\ShortUrlDataTransformer::class, - Visit\Transformer\OrphanVisitDataTransformer::class, - ], + EventDispatcher\PublishingUpdatesGenerator::class => [ShortUrl\Transformer\ShortUrlDataTransformer::class], Importer\ImportedLinksProcessor::class => [ 'em', diff --git a/module/Core/src/EventDispatcher/PublishingUpdatesGenerator.php b/module/Core/src/EventDispatcher/PublishingUpdatesGenerator.php index 06d06c84..82ada6e1 100644 --- a/module/Core/src/EventDispatcher/PublishingUpdatesGenerator.php +++ b/module/Core/src/EventDispatcher/PublishingUpdatesGenerator.php @@ -9,12 +9,10 @@ use Shlinkio\Shlink\Common\UpdatePublishing\Update; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; use Shlinkio\Shlink\Core\Visit\Entity\Visit; -final class PublishingUpdatesGenerator implements PublishingUpdatesGeneratorInterface +final readonly class PublishingUpdatesGenerator implements PublishingUpdatesGeneratorInterface { - public function __construct( - private readonly DataTransformerInterface $shortUrlTransformer, - private readonly DataTransformerInterface $orphanVisitTransformer, - ) { + public function __construct(private DataTransformerInterface $shortUrlTransformer) + { } public function newVisitUpdate(Visit $visit): Update @@ -28,7 +26,7 @@ final class PublishingUpdatesGenerator implements PublishingUpdatesGeneratorInte public function newOrphanVisitUpdate(Visit $visit): Update { return Update::forTopicAndPayload(Topic::NEW_ORPHAN_VISIT->value, [ - 'visit' => $this->orphanVisitTransformer->transform($visit), + 'visit' => $visit->jsonSerialize(), ]); } diff --git a/module/Core/src/Visit/Transformer/OrphanVisitDataTransformer.php b/module/Core/src/Visit/Transformer/OrphanVisitDataTransformer.php deleted file mode 100644 index 79e8f839..00000000 --- a/module/Core/src/Visit/Transformer/OrphanVisitDataTransformer.php +++ /dev/null @@ -1,19 +0,0 @@ -jsonSerialize(); - } -} diff --git a/module/Core/test/EventDispatcher/PublishingUpdatesGeneratorTest.php b/module/Core/test/EventDispatcher/PublishingUpdatesGeneratorTest.php index 94802cae..36fd6c8f 100644 --- a/module/Core/test/EventDispatcher/PublishingUpdatesGeneratorTest.php +++ b/module/Core/test/EventDispatcher/PublishingUpdatesGeneratorTest.php @@ -18,7 +18,6 @@ use Shlinkio\Shlink\Core\Visit\Entity\Visit; use Shlinkio\Shlink\Core\Visit\Model\Visitor; use Shlinkio\Shlink\Core\Visit\Model\VisitsSummary; use Shlinkio\Shlink\Core\Visit\Model\VisitType; -use Shlinkio\Shlink\Core\Visit\Transformer\OrphanVisitDataTransformer; class PublishingUpdatesGeneratorTest extends TestCase { @@ -28,7 +27,6 @@ class PublishingUpdatesGeneratorTest extends TestCase { $this->generator = new PublishingUpdatesGenerator( new ShortUrlDataTransformer(new ShortUrlStringifier([])), - new OrphanVisitDataTransformer(), ); } diff --git a/module/Core/test/Visit/Entity/VisitTest.php b/module/Core/test/Visit/Entity/VisitTest.php index 3fea2882..62c56b2e 100644 --- a/module/Core/test/Visit/Entity/VisitTest.php +++ b/module/Core/test/Visit/Entity/VisitTest.php @@ -4,13 +4,18 @@ declare(strict_types=1); namespace ShlinkioTest\Shlink\Core\Visit\Entity; +use Laminas\Diactoros\ServerRequestFactory; +use Laminas\Diactoros\Uri; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\TestCase; use Shlinkio\Shlink\Common\Util\IpAddress; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; use Shlinkio\Shlink\Core\Visit\Entity\Visit; +use Shlinkio\Shlink\Core\Visit\Entity\VisitLocation; use Shlinkio\Shlink\Core\Visit\Model\Visitor; +use Shlinkio\Shlink\Core\Visit\Model\VisitType; +use Shlinkio\Shlink\IpGeolocation\Model\Location; class VisitTest extends TestCase { @@ -40,6 +45,62 @@ class VisitTest extends TestCase yield 'Guzzle' => ['guzzlehttp', true]; } + #[Test, DataProvider('provideOrphanVisits')] + public function isProperlyJsonSerializedWhenOrphan(Visit $visit, array $expectedResult): void + { + self::assertEquals($expectedResult, $visit->jsonSerialize()); + } + + public static function provideOrphanVisits(): iterable + { + yield 'base path visit' => [ + $visit = Visit::forBasePath(Visitor::emptyInstance()), + [ + 'referer' => '', + 'date' => $visit->getDate()->toAtomString(), + 'userAgent' => '', + 'visitLocation' => null, + 'potentialBot' => false, + 'visitedUrl' => '', + 'type' => VisitType::BASE_URL->value, + ], + ]; + yield 'invalid short url visit' => [ + $visit = Visit::forInvalidShortUrl(Visitor::fromRequest( + ServerRequestFactory::fromGlobals()->withHeader('User-Agent', 'foo') + ->withHeader('Referer', 'bar') + ->withUri(new Uri('https://example.com/foo')), + )), + [ + 'referer' => 'bar', + 'date' => $visit->getDate()->toAtomString(), + 'userAgent' => 'foo', + 'visitLocation' => null, + 'potentialBot' => false, + 'visitedUrl' => 'https://example.com/foo', + 'type' => VisitType::INVALID_SHORT_URL->value, + ], + ]; + yield 'regular 404 visit' => [ + $visit = Visit::forRegularNotFound( + Visitor::fromRequest( + ServerRequestFactory::fromGlobals()->withHeader('User-Agent', 'user-agent') + ->withHeader('Referer', 'referer') + ->withUri(new Uri('https://s.test/foo/bar')), + ), + )->locate($location = VisitLocation::fromGeolocation(Location::emptyInstance())), + [ + 'referer' => 'referer', + 'date' => $visit->getDate()->toAtomString(), + 'userAgent' => 'user-agent', + 'visitLocation' => $location, + 'potentialBot' => false, + 'visitedUrl' => 'https://s.test/foo/bar', + 'type' => VisitType::REGULAR_404->value, + ], + ]; + } + #[Test, DataProvider('provideAddresses')] public function addressIsAnonymizedWhenRequested(bool $anonymize, ?string $address, ?string $expectedAddress): void { diff --git a/module/Core/test/Visit/Transformer/OrphanVisitDataTransformerTest.php b/module/Core/test/Visit/Transformer/OrphanVisitDataTransformerTest.php deleted file mode 100644 index 527f4fc9..00000000 --- a/module/Core/test/Visit/Transformer/OrphanVisitDataTransformerTest.php +++ /dev/null @@ -1,85 +0,0 @@ -transformer = new OrphanVisitDataTransformer(); - } - - #[Test, DataProvider('provideVisits')] - public function visitsAreParsedAsExpected(Visit $visit, array $expectedResult): void - { - $result = $this->transformer->transform($visit); - - self::assertEquals($expectedResult, $result); - } - - public static function provideVisits(): iterable - { - yield 'base path visit' => [ - $visit = Visit::forBasePath(Visitor::emptyInstance()), - [ - 'referer' => '', - 'date' => $visit->getDate()->toAtomString(), - 'userAgent' => '', - 'visitLocation' => null, - 'potentialBot' => false, - 'visitedUrl' => '', - 'type' => VisitType::BASE_URL->value, - ], - ]; - yield 'invalid short url visit' => [ - $visit = Visit::forInvalidShortUrl(Visitor::fromRequest( - ServerRequestFactory::fromGlobals()->withHeader('User-Agent', 'foo') - ->withHeader('Referer', 'bar') - ->withUri(new Uri('https://example.com/foo')), - )), - [ - 'referer' => 'bar', - 'date' => $visit->getDate()->toAtomString(), - 'userAgent' => 'foo', - 'visitLocation' => null, - 'potentialBot' => false, - 'visitedUrl' => 'https://example.com/foo', - 'type' => VisitType::INVALID_SHORT_URL->value, - ], - ]; - yield 'regular 404 visit' => [ - $visit = Visit::forRegularNotFound( - Visitor::fromRequest( - ServerRequestFactory::fromGlobals()->withHeader('User-Agent', 'user-agent') - ->withHeader('Referer', 'referer') - ->withUri(new Uri('https://s.test/foo/bar')), - ), - )->locate($location = VisitLocation::fromGeolocation(Location::emptyInstance())), - [ - 'referer' => 'referer', - 'date' => $visit->getDate()->toAtomString(), - 'userAgent' => 'user-agent', - 'visitLocation' => $location, - 'potentialBot' => false, - 'visitedUrl' => 'https://s.test/foo/bar', - 'type' => VisitType::REGULAR_404->value, - ], - ]; - } -} diff --git a/module/Rest/config/dependencies.config.php b/module/Rest/config/dependencies.config.php index 9396dd38..d334d5b0 100644 --- a/module/Rest/config/dependencies.config.php +++ b/module/Rest/config/dependencies.config.php @@ -89,10 +89,7 @@ return [ 'config.url_shortener.domain.hostname', ], Action\Visit\GlobalVisitsAction::class => [Visit\VisitsStatsHelper::class], - Action\Visit\OrphanVisitsAction::class => [ - Visit\VisitsStatsHelper::class, - Visit\Transformer\OrphanVisitDataTransformer::class, - ], + Action\Visit\OrphanVisitsAction::class => [Visit\VisitsStatsHelper::class], Action\Visit\DeleteOrphanVisitsAction::class => [Visit\VisitsDeleter::class], Action\Visit\NonOrphanVisitsAction::class => [Visit\VisitsStatsHelper::class], Action\ShortUrl\ListShortUrlsAction::class => [ diff --git a/module/Rest/src/Action/Visit/OrphanVisitsAction.php b/module/Rest/src/Action/Visit/OrphanVisitsAction.php index 57244197..0224022d 100644 --- a/module/Rest/src/Action/Visit/OrphanVisitsAction.php +++ b/module/Rest/src/Action/Visit/OrphanVisitsAction.php @@ -8,7 +8,6 @@ use Laminas\Diactoros\Response\JsonResponse; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; use Shlinkio\Shlink\Common\Paginator\Util\PagerfantaUtilsTrait; -use Shlinkio\Shlink\Common\Rest\DataTransformerInterface; use Shlinkio\Shlink\Core\Visit\Model\OrphanVisitsParams; use Shlinkio\Shlink\Core\Visit\VisitsStatsHelperInterface; use Shlinkio\Shlink\Rest\Action\AbstractRestAction; @@ -21,10 +20,8 @@ class OrphanVisitsAction extends AbstractRestAction protected const ROUTE_PATH = '/visits/orphan'; protected const ROUTE_ALLOWED_METHODS = [self::METHOD_GET]; - public function __construct( - private readonly VisitsStatsHelperInterface $visitsHelper, - private readonly DataTransformerInterface $orphanVisitTransformer, - ) { + public function __construct(private readonly VisitsStatsHelperInterface $visitsHelper) + { } public function handle(ServerRequestInterface $request): ResponseInterface @@ -34,7 +31,7 @@ class OrphanVisitsAction extends AbstractRestAction $visits = $this->visitsHelper->orphanVisits($params, $apiKey); return new JsonResponse([ - 'visits' => $this->serializePaginator($visits, $this->orphanVisitTransformer), + 'visits' => $this->serializePaginator($visits), ]); } } diff --git a/module/Rest/test/Action/Visit/OrphanVisitsActionTest.php b/module/Rest/test/Action/Visit/OrphanVisitsActionTest.php index efa14caa..d5bdfef9 100644 --- a/module/Rest/test/Action/Visit/OrphanVisitsActionTest.php +++ b/module/Rest/test/Action/Visit/OrphanVisitsActionTest.php @@ -11,7 +11,6 @@ use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Shlinkio\Shlink\Common\Paginator\Paginator; -use Shlinkio\Shlink\Common\Rest\DataTransformerInterface; use Shlinkio\Shlink\Core\Exception\ValidationException; use Shlinkio\Shlink\Core\Visit\Entity\Visit; use Shlinkio\Shlink\Core\Visit\Model\OrphanVisitsParams; @@ -26,14 +25,11 @@ class OrphanVisitsActionTest extends TestCase { private OrphanVisitsAction $action; private MockObject & VisitsStatsHelperInterface $visitsHelper; - private MockObject & DataTransformerInterface $orphanVisitTransformer; protected function setUp(): void { $this->visitsHelper = $this->createMock(VisitsStatsHelperInterface::class); - $this->orphanVisitTransformer = $this->createMock(DataTransformerInterface::class); - - $this->action = new OrphanVisitsAction($this->visitsHelper, $this->orphanVisitTransformer); + $this->action = new OrphanVisitsAction($this->visitsHelper); } #[Test] @@ -45,9 +41,6 @@ class OrphanVisitsActionTest extends TestCase $this->isInstanceOf(OrphanVisitsParams::class), )->willReturn(new Paginator(new ArrayAdapter($visits))); $visitsAmount = count($visits); - $this->orphanVisitTransformer->expects($this->exactly($visitsAmount))->method('transform')->with( - $this->isInstanceOf(Visit::class), - )->willReturn([]); /** @var JsonResponse $response */ $response = $this->action->handle(