diff --git a/CHANGELOG.md b/CHANGELOG.md index 74ee51ab..947806a0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,32 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com), and this project adheres to [Semantic Versioning](https://semver.org). +## 2.0.4 - 2020-02-02 + +#### Added + +* *Nothing* + +#### Changed + +* [#577](https://github.com/shlinkio/shlink/issues/577) Wrapped params used to customize short URL lists into a DTO with implicit validation. + +#### Deprecated + +* *Nothing* + +#### Removed + +* *Nothing* + +#### Fixed + +* [#620](https://github.com/shlinkio/shlink/issues/620) Ensured "controlled" errors (like validation errors and such) won't be logged with error level, preventing logs to be polluted. +* [#637](https://github.com/shlinkio/shlink/issues/637) Fixed several work flows in which short URLs with domain are handled form the API. +* [#644](https://github.com/shlinkio/shlink/issues/644) Fixed visits to short URL on non-default domain being linked to the URL on default domain with the same short code. +* [#643](https://github.com/shlinkio/shlink/issues/643) Fixed searching on short URL lists not taking into consideration the domain name. + + ## 2.0.3 - 2020-01-27 #### Added diff --git a/composer.json b/composer.json index 19b3f557..908b9af3 100644 --- a/composer.json +++ b/composer.json @@ -47,7 +47,7 @@ "phly/phly-event-dispatcher": "^1.0", "predis/predis": "^1.1", "pugx/shortid-php": "^0.5", - "shlinkio/shlink-common": "^2.5", + "shlinkio/shlink-common": "^2.7.0", "shlinkio/shlink-event-dispatcher": "^1.3", "shlinkio/shlink-installer": "^4.0.1", "shlinkio/shlink-ip-geolocation": "^1.3.1", diff --git a/config/autoload/entity-manager.global.php b/config/autoload/entity-manager.global.php index 561579f1..c08f66f2 100644 --- a/config/autoload/entity-manager.global.php +++ b/config/autoload/entity-manager.global.php @@ -9,6 +9,7 @@ return [ 'entity_manager' => [ 'orm' => [ 'proxies_dir' => 'data/proxies', + 'load_mappings_using_functional_style' => true, ], 'connection' => [ 'user' => '', diff --git a/docs/swagger/definitions/ShortUrl.json b/docs/swagger/definitions/ShortUrl.json index 8111bd6e..66d20115 100644 --- a/docs/swagger/definitions/ShortUrl.json +++ b/docs/swagger/definitions/ShortUrl.json @@ -31,6 +31,10 @@ }, "meta": { "$ref": "./ShortUrlMeta.json" + }, + "domain": { + "type": "string", + "description": "The domain in which the short URL was created. Null if it belongs to default domain." } } } diff --git a/docs/swagger/parameters/domain.json b/docs/swagger/parameters/domain.json new file mode 100644 index 00000000..9a9b41b9 --- /dev/null +++ b/docs/swagger/parameters/domain.json @@ -0,0 +1,9 @@ +{ + "name": "domain", + "description": "The domain in which the short code should be searched for.", + "in": "query", + "required": false, + "schema": { + "type": "string" + } +} diff --git a/docs/swagger/paths/v1_short-urls.json b/docs/swagger/paths/v1_short-urls.json index 0c6d0484..be274ab6 100644 --- a/docs/swagger/paths/v1_short-urls.json +++ b/docs/swagger/paths/v1_short-urls.json @@ -123,7 +123,8 @@ "validSince": "2017-01-21T00:00:00+02:00", "validUntil": null, "maxVisits": 100 - } + }, + "domain": null }, { "shortCode": "12Kb3", @@ -138,11 +139,12 @@ "validSince": null, "validUntil": null, "maxVisits": null - } + }, + "domain": null }, { "shortCode": "123bA", - "shortUrl": "https://doma.in/123bA", + "shortUrl": "https://example.com/123bA", "longUrl": "https://www.google.com", "dateCreated": "2015-10-01T20:34:16+02:00", "visitsCount": 25, @@ -151,7 +153,8 @@ "validSince": "2017-01-21T00:00:00+02:00", "validUntil": null, "maxVisits": null - } + }, + "domain": "example.com" } ], "pagination": { @@ -271,7 +274,8 @@ "validSince": "2017-01-21T00:00:00+02:00", "validUntil": null, "maxVisits": 500 - } + }, + "domain": null } } }, diff --git a/docs/swagger/paths/v1_short-urls_shorten.json b/docs/swagger/paths/v1_short-urls_shorten.json index c5ad9352..c31c0cd9 100644 --- a/docs/swagger/paths/v1_short-urls_shorten.json +++ b/docs/swagger/paths/v1_short-urls_shorten.json @@ -72,7 +72,8 @@ "validSince": "2017-01-21T00:00:00+02:00", "validUntil": null, "maxVisits": 100 - } + }, + "domain": null }, "text/plain": "https://doma.in/abc123" } diff --git a/docs/swagger/paths/v1_short-urls_{shortCode}.json b/docs/swagger/paths/v1_short-urls_{shortCode}.json index d4537a83..b9baad92 100644 --- a/docs/swagger/paths/v1_short-urls_{shortCode}.json +++ b/docs/swagger/paths/v1_short-urls_{shortCode}.json @@ -20,13 +20,7 @@ } }, { - "name": "domain", - "in": "query", - "description": "The domain in which the short code should be searched for. Will fall back to default domain if not found.", - "required": false, - "schema": { - "type": "string" - } + "$ref": "../parameters/domain.json" } ], "security": [ @@ -58,7 +52,8 @@ "validSince": "2017-01-21T00:00:00+02:00", "validUntil": null, "maxVisits": 100 - } + }, + "domain": null } } }, @@ -104,6 +99,9 @@ "schema": { "type": "string" } + }, + { + "$ref": "../parameters/domain.json" } ], "requestBody": { @@ -214,6 +212,9 @@ "schema": { "type": "string" } + }, + { + "$ref": "../parameters/domain.json" } ], "security": [ diff --git a/docs/swagger/paths/v1_short-urls_{shortCode}_tags.json b/docs/swagger/paths/v1_short-urls_{shortCode}_tags.json index 4065f718..fd497380 100644 --- a/docs/swagger/paths/v1_short-urls_{shortCode}_tags.json +++ b/docs/swagger/paths/v1_short-urls_{shortCode}_tags.json @@ -18,6 +18,9 @@ "schema": { "type": "string" } + }, + { + "$ref": "../parameters/domain.json" } ], "requestBody": { diff --git a/docs/swagger/paths/v1_short-urls_{shortCode}_visits.json b/docs/swagger/paths/v1_short-urls_{shortCode}_visits.json index 2369ba13..03d66a99 100644 --- a/docs/swagger/paths/v1_short-urls_{shortCode}_visits.json +++ b/docs/swagger/paths/v1_short-urls_{shortCode}_visits.json @@ -19,6 +19,9 @@ "type": "string" } }, + { + "$ref": "../parameters/domain.json" + }, { "name": "startDate", "in": "query", diff --git a/module/CLI/src/Command/ShortUrl/DeleteShortUrlCommand.php b/module/CLI/src/Command/ShortUrl/DeleteShortUrlCommand.php index bee66c34..f57001b0 100644 --- a/module/CLI/src/Command/ShortUrl/DeleteShortUrlCommand.php +++ b/module/CLI/src/Command/ShortUrl/DeleteShortUrlCommand.php @@ -6,6 +6,7 @@ namespace Shlinkio\Shlink\CLI\Command\ShortUrl; use Shlinkio\Shlink\CLI\Util\ExitCodes; use Shlinkio\Shlink\Core\Exception; +use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Service\ShortUrl\DeleteShortUrlServiceInterface; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputArgument; @@ -40,33 +41,39 @@ class DeleteShortUrlCommand extends Command InputOption::VALUE_NONE, 'Ignores the safety visits threshold check, which could make short URLs with many visits to be ' . 'accidentally deleted', + ) + ->addOption( + 'domain', + 'd', + InputOption::VALUE_REQUIRED, + 'The domain if the short code does not belong to the default one', ); } protected function execute(InputInterface $input, OutputInterface $output): ?int { $io = new SymfonyStyle($input, $output); - $shortCode = $input->getArgument('shortCode'); + $identifier = ShortUrlIdentifier::fromCli($input); $ignoreThreshold = $input->getOption('ignore-threshold'); try { - $this->runDelete($io, $shortCode, $ignoreThreshold); + $this->runDelete($io, $identifier, $ignoreThreshold); return ExitCodes::EXIT_SUCCESS; } catch (Exception\ShortUrlNotFoundException $e) { $io->error($e->getMessage()); return ExitCodes::EXIT_FAILURE; } catch (Exception\DeleteShortUrlException $e) { - return $this->retry($io, $shortCode, $e->getMessage()); + return $this->retry($io, $identifier, $e->getMessage()); } } - private function retry(SymfonyStyle $io, string $shortCode, string $warningMsg): int + private function retry(SymfonyStyle $io, ShortUrlIdentifier $identifier, string $warningMsg): int { $io->writeln(sprintf('%s', $warningMsg)); $forceDelete = $io->confirm('Do you want to delete it anyway?', false); if ($forceDelete) { - $this->runDelete($io, $shortCode, true); + $this->runDelete($io, $identifier, true); } else { $io->warning('Short URL was not deleted.'); } @@ -74,9 +81,9 @@ class DeleteShortUrlCommand extends Command return $forceDelete ? ExitCodes::EXIT_SUCCESS : ExitCodes::EXIT_WARNING; } - private function runDelete(SymfonyStyle $io, string $shortCode, bool $ignoreThreshold): void + private function runDelete(SymfonyStyle $io, ShortUrlIdentifier $identifier, bool $ignoreThreshold): void { - $this->deleteShortUrlService->deleteByShortCode($shortCode, $ignoreThreshold); - $io->success(sprintf('Short URL with short code "%s" successfully deleted.', $shortCode)); + $this->deleteShortUrlService->deleteByShortCode($identifier, $ignoreThreshold); + $io->success(sprintf('Short URL with short code "%s" successfully deleted.', $identifier->shortCode())); } } diff --git a/module/CLI/src/Command/ShortUrl/GetVisitsCommand.php b/module/CLI/src/Command/ShortUrl/GetVisitsCommand.php index 363ff3aa..43949993 100644 --- a/module/CLI/src/Command/ShortUrl/GetVisitsCommand.php +++ b/module/CLI/src/Command/ShortUrl/GetVisitsCommand.php @@ -9,10 +9,12 @@ use Shlinkio\Shlink\CLI\Util\ExitCodes; use Shlinkio\Shlink\CLI\Util\ShlinkTable; use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\Entity\Visit; +use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Model\VisitsParams; use Shlinkio\Shlink\Core\Service\VisitsTrackerInterface; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; +use Symfony\Component\Console\Input\InputOption; use Symfony\Component\Console\Output\OutputInterface; use Symfony\Component\Console\Style\SymfonyStyle; @@ -36,7 +38,8 @@ class GetVisitsCommand extends AbstractWithDateRangeCommand $this ->setName(self::NAME) ->setDescription('Returns the detailed visits information for provided short code') - ->addArgument('shortCode', InputArgument::REQUIRED, 'The short code which visits we want to get'); + ->addArgument('shortCode', InputArgument::REQUIRED, 'The short code which visits we want to get') + ->addOption('domain', 'd', InputOption::VALUE_REQUIRED, 'The domain for the short code'); } protected function getStartDateDesc(): string @@ -65,11 +68,11 @@ class GetVisitsCommand extends AbstractWithDateRangeCommand protected function execute(InputInterface $input, OutputInterface $output): ?int { - $shortCode = $input->getArgument('shortCode'); + $identifier = ShortUrlIdentifier::fromCli($input); $startDate = $this->getDateOption($input, $output, 'startDate'); $endDate = $this->getDateOption($input, $output, 'endDate'); - $paginator = $this->visitsTracker->info($shortCode, new VisitsParams(new DateRange($startDate, $endDate))); + $paginator = $this->visitsTracker->info($identifier, new VisitsParams(new DateRange($startDate, $endDate))); $rows = map($paginator->getCurrentItems(), function (Visit $visit) { $rowData = $visit->jsonSerialize(); diff --git a/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php b/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php index c4251ed9..98f006d4 100644 --- a/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php +++ b/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php @@ -4,16 +4,17 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI\Command\ShortUrl; -use Cake\Chronos\Chronos; use Laminas\Paginator\Paginator; use Shlinkio\Shlink\CLI\Command\Util\AbstractWithDateRangeCommand; use Shlinkio\Shlink\CLI\Util\ExitCodes; use Shlinkio\Shlink\CLI\Util\ShlinkTable; use Shlinkio\Shlink\Common\Paginator\Util\PaginatorUtilsTrait; -use Shlinkio\Shlink\Common\Util\DateRange; +use Shlinkio\Shlink\Core\Model\ShortUrlsOrdering; +use Shlinkio\Shlink\Core\Model\ShortUrlsParams; use Shlinkio\Shlink\Core\Paginator\Adapter\ShortUrlRepositoryAdapter; use Shlinkio\Shlink\Core\Service\ShortUrlServiceInterface; use Shlinkio\Shlink\Core\Transformer\ShortUrlDataTransformer; +use Shlinkio\Shlink\Core\Validation\ShortUrlsParamsInputFilter; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Input\InputOption; use Symfony\Component\Console\Output\OutputInterface; @@ -108,7 +109,14 @@ class ListShortUrlsCommand extends AbstractWithDateRangeCommand $orderBy = $this->processOrderBy($input); do { - $result = $this->renderPage($output, $page, $searchTerm, $tags, $showTags, $startDate, $endDate, $orderBy); + $result = $this->renderPage($output, $showTags, ShortUrlsParams::fromRawData([ + ShortUrlsParamsInputFilter::PAGE => $page, + ShortUrlsParamsInputFilter::SEARCH_TERM => $searchTerm, + ShortUrlsParamsInputFilter::TAGS => $tags, + ShortUrlsOrdering::ORDER_BY => $orderBy, + ShortUrlsParamsInputFilter::START_DATE => $startDate !== null ? $startDate->toAtomString() : null, + ShortUrlsParamsInputFilter::END_DATE => $endDate !== null ? $endDate->toAtomString() : null, + ])); $page++; $continue = $this->isLastPage($result) @@ -122,26 +130,9 @@ class ListShortUrlsCommand extends AbstractWithDateRangeCommand return ExitCodes::EXIT_SUCCESS; } - /** - * @param string|array|null $orderBy - */ - private function renderPage( - OutputInterface $output, - int $page, - ?string $searchTerm, - array $tags, - bool $showTags, - ?Chronos $startDate, - ?Chronos $endDate, - $orderBy - ): Paginator { - $result = $this->shortUrlService->listShortUrls( - $page, - $searchTerm, - $tags, - $orderBy, - new DateRange($startDate, $endDate), - ); + private function renderPage(OutputInterface $output, bool $showTags, ShortUrlsParams $params): Paginator + { + $result = $this->shortUrlService->listShortUrls($params); $headers = ['Short code', 'Short URL', 'Long URL', 'Date created', 'Visits count']; if ($showTags) { diff --git a/module/CLI/src/Command/ShortUrl/ResolveUrlCommand.php b/module/CLI/src/Command/ShortUrl/ResolveUrlCommand.php index 22f384db..e6fdef3d 100644 --- a/module/CLI/src/Command/ShortUrl/ResolveUrlCommand.php +++ b/module/CLI/src/Command/ShortUrl/ResolveUrlCommand.php @@ -6,6 +6,7 @@ namespace Shlinkio\Shlink\CLI\Command\ShortUrl; use Shlinkio\Shlink\CLI\Util\ExitCodes; use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; +use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Service\ShortUrl\ShortUrlResolverInterface; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputArgument; @@ -54,11 +55,9 @@ class ResolveUrlCommand extends Command protected function execute(InputInterface $input, OutputInterface $output): ?int { $io = new SymfonyStyle($input, $output); - $shortCode = $input->getArgument('shortCode'); - $domain = $input->getOption('domain'); try { - $url = $this->urlResolver->shortCodeToShortUrl($shortCode, $domain); + $url = $this->urlResolver->resolveShortUrl(ShortUrlIdentifier::fromCli($input)); $output->writeln(sprintf('Long URL: %s', $url->getLongUrl())); return ExitCodes::EXIT_SUCCESS; } catch (ShortUrlNotFoundException $e) { diff --git a/module/CLI/test/Command/ShortUrl/DeleteShortUrlCommandTest.php b/module/CLI/test/Command/ShortUrl/DeleteShortUrlCommandTest.php index 7fd727ca..2c3526f5 100644 --- a/module/CLI/test/Command/ShortUrl/DeleteShortUrlCommandTest.php +++ b/module/CLI/test/Command/ShortUrl/DeleteShortUrlCommandTest.php @@ -9,6 +9,7 @@ use Prophecy\Argument; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\CLI\Command\ShortUrl\DeleteShortUrlCommand; use Shlinkio\Shlink\Core\Exception; +use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Service\ShortUrl\DeleteShortUrlServiceInterface; use Symfony\Component\Console\Application; use Symfony\Component\Console\Tester\CommandTester; @@ -38,8 +39,10 @@ class DeleteShortUrlCommandTest extends TestCase public function successMessageIsPrintedIfUrlIsProperlyDeleted(): void { $shortCode = 'abc123'; - $deleteByShortCode = $this->service->deleteByShortCode($shortCode, false)->will(function (): void { - }); + $deleteByShortCode = $this->service->deleteByShortCode(new ShortUrlIdentifier($shortCode), false)->will( + function (): void { + }, + ); $this->commandTester->execute(['shortCode' => $shortCode]); $output = $this->commandTester->getDisplay(); @@ -55,8 +58,9 @@ class DeleteShortUrlCommandTest extends TestCase public function invalidShortCodePrintsMessage(): void { $shortCode = 'abc123'; - $deleteByShortCode = $this->service->deleteByShortCode($shortCode, false)->willThrow( - Exception\ShortUrlNotFoundException::fromNotFoundShortCode($shortCode), + $identifier = new ShortUrlIdentifier($shortCode); + $deleteByShortCode = $this->service->deleteByShortCode($identifier, false)->willThrow( + Exception\ShortUrlNotFoundException::fromNotFound($identifier), ); $this->commandTester->execute(['shortCode' => $shortCode]); @@ -76,7 +80,8 @@ class DeleteShortUrlCommandTest extends TestCase string $expectedMessage ): void { $shortCode = 'abc123'; - $deleteByShortCode = $this->service->deleteByShortCode($shortCode, Argument::type('bool'))->will( + $identifier = new ShortUrlIdentifier($shortCode); + $deleteByShortCode = $this->service->deleteByShortCode($identifier, Argument::type('bool'))->will( function (array $args) use ($shortCode): void { $ignoreThreshold = array_pop($args); @@ -109,7 +114,7 @@ class DeleteShortUrlCommandTest extends TestCase public function deleteIsNotRetriedWhenThresholdIsReachedAndQuestionIsDeclined(): void { $shortCode = 'abc123'; - $deleteByShortCode = $this->service->deleteByShortCode($shortCode, false)->willThrow( + $deleteByShortCode = $this->service->deleteByShortCode(new ShortUrlIdentifier($shortCode), false)->willThrow( Exception\DeleteShortUrlException::fromVisitsThreshold(10, $shortCode), ); $this->commandTester->setInputs(['no']); diff --git a/module/CLI/test/Command/ShortUrl/GetVisitsCommandTest.php b/module/CLI/test/Command/ShortUrl/GetVisitsCommandTest.php index 53ba114e..a725240e 100644 --- a/module/CLI/test/Command/ShortUrl/GetVisitsCommandTest.php +++ b/module/CLI/test/Command/ShortUrl/GetVisitsCommandTest.php @@ -15,6 +15,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\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Model\Visitor; use Shlinkio\Shlink\Core\Model\VisitsParams; use Shlinkio\Shlink\Core\Service\VisitsTrackerInterface; @@ -42,9 +43,12 @@ class GetVisitsCommandTest extends TestCase public function noDateFlagsTriesToListWithoutDateRange(): void { $shortCode = 'abc123'; - $this->visitsTracker->info($shortCode, new VisitsParams(new DateRange(null, null)))->willReturn( - new Paginator(new ArrayAdapter([])), - )->shouldBeCalledOnce(); + $this->visitsTracker->info( + new ShortUrlIdentifier($shortCode), + new VisitsParams(new DateRange(null, null)), + ) + ->willReturn(new Paginator(new ArrayAdapter([]))) + ->shouldBeCalledOnce(); $this->commandTester->execute(['shortCode' => $shortCode]); } @@ -56,7 +60,7 @@ class GetVisitsCommandTest extends TestCase $startDate = '2016-01-01'; $endDate = '2016-02-01'; $this->visitsTracker->info( - $shortCode, + new ShortUrlIdentifier($shortCode), new VisitsParams(new DateRange(Chronos::parse($startDate), Chronos::parse($endDate))), ) ->willReturn(new Paginator(new ArrayAdapter([]))) @@ -74,7 +78,7 @@ class GetVisitsCommandTest extends TestCase { $shortCode = 'abc123'; $startDate = 'foo'; - $info = $this->visitsTracker->info($shortCode, new VisitsParams(new DateRange())) + $info = $this->visitsTracker->info(new ShortUrlIdentifier($shortCode), new VisitsParams(new DateRange())) ->willReturn(new Paginator(new ArrayAdapter([]))); $this->commandTester->execute([ @@ -94,7 +98,7 @@ class GetVisitsCommandTest extends TestCase public function outputIsProperlyGenerated(): void { $shortCode = 'abc123'; - $this->visitsTracker->info($shortCode, Argument::any())->willReturn( + $this->visitsTracker->info(new ShortUrlIdentifier($shortCode), Argument::any())->willReturn( new Paginator(new ArrayAdapter([ (new Visit(new ShortUrl(''), new Visitor('bar', 'foo', '')))->locate( new VisitLocation(new Location('', 'Spain', '', '', 0, 0, '')), diff --git a/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php b/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php index d4182e27..2e315581 100644 --- a/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php +++ b/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php @@ -11,8 +11,8 @@ use PHPUnit\Framework\TestCase; use Prophecy\Argument; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\CLI\Command\ShortUrl\ListShortUrlsCommand; -use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\Entity\ShortUrl; +use Shlinkio\Shlink\Core\Model\ShortUrlsParams; use Shlinkio\Shlink\Core\Service\ShortUrlServiceInterface; use Symfony\Component\Console\Application; use Symfony\Component\Console\Tester\CommandTester; @@ -64,7 +64,7 @@ class ListShortUrlsCommandTest extends TestCase $data[] = new ShortUrl('url_' . $i); } - $this->shortUrlService->listShortUrls(1, null, [], null, new DateRange()) + $this->shortUrlService->listShortUrls(ShortUrlsParams::emptyInstance()) ->willReturn(new Paginator(new ArrayAdapter($data))) ->shouldBeCalledOnce(); @@ -85,7 +85,7 @@ class ListShortUrlsCommandTest extends TestCase public function passingPageWillMakeListStartOnThatPage(): void { $page = 5; - $this->shortUrlService->listShortUrls($page, null, [], null, new DateRange()) + $this->shortUrlService->listShortUrls(ShortUrlsParams::fromRawData(['page' => $page])) ->willReturn(new Paginator(new ArrayAdapter())) ->shouldBeCalledOnce(); @@ -96,7 +96,7 @@ class ListShortUrlsCommandTest extends TestCase /** @test */ public function ifTagsFlagIsProvidedTagsColumnIsIncluded(): void { - $this->shortUrlService->listShortUrls(1, null, [], null, new DateRange()) + $this->shortUrlService->listShortUrls(ShortUrlsParams::emptyInstance()) ->willReturn(new Paginator(new ArrayAdapter())) ->shouldBeCalledOnce(); @@ -115,10 +115,16 @@ class ListShortUrlsCommandTest extends TestCase ?int $page, ?string $searchTerm, array $tags, - ?DateRange $dateRange + ?string $startDate = null, + ?string $endDate = null ): void { - $listShortUrls = $this->shortUrlService->listShortUrls($page, $searchTerm, $tags, null, $dateRange) - ->willReturn(new Paginator(new ArrayAdapter())); + $listShortUrls = $this->shortUrlService->listShortUrls(ShortUrlsParams::fromRawData([ + 'page' => $page, + 'searchTerm' => $searchTerm, + 'tags' => $tags, + 'startDate' => $startDate !== null ? Chronos::parse($startDate)->toAtomString() : null, + 'endDate' => $endDate !== null ? Chronos::parse($endDate)->toAtomString() : null, + ]))->willReturn(new Paginator(new ArrayAdapter())); $this->commandTester->setInputs(['n']); $this->commandTester->execute($commandArgs); @@ -128,36 +134,37 @@ class ListShortUrlsCommandTest extends TestCase public function provideArgs(): iterable { - yield [[], 1, null, [], new DateRange()]; - yield [['--page' => $page = 3], $page, null, [], new DateRange()]; - yield [['--searchTerm' => $searchTerm = 'search this'], 1, $searchTerm, [], new DateRange()]; + yield [[], 1, null, []]; + yield [['--page' => $page = 3], $page, null, []]; + yield [['--searchTerm' => $searchTerm = 'search this'], 1, $searchTerm, []]; yield [ ['--page' => $page = 3, '--searchTerm' => $searchTerm = 'search this', '--tags' => $tags = 'foo,bar'], $page, $searchTerm, explode(',', $tags), - new DateRange(), ]; yield [ ['--startDate' => $startDate = '2019-01-01'], 1, null, [], - new DateRange(Chronos::parse($startDate)), + $startDate, ]; yield [ ['--endDate' => $endDate = '2020-05-23'], 1, null, [], - new DateRange(null, Chronos::parse($endDate)), + null, + $endDate, ]; yield [ ['--startDate' => $startDate = '2019-01-01', '--endDate' => $endDate = '2020-05-23'], 1, null, [], - new DateRange(Chronos::parse($startDate), Chronos::parse($endDate)), + $startDate, + $endDate, ]; } @@ -168,8 +175,9 @@ class ListShortUrlsCommandTest extends TestCase */ public function orderByIsProperlyComputed(array $commandArgs, $expectedOrderBy): void { - $listShortUrls = $this->shortUrlService->listShortUrls(1, null, [], $expectedOrderBy, new DateRange()) - ->willReturn(new Paginator(new ArrayAdapter())); + $listShortUrls = $this->shortUrlService->listShortUrls(ShortUrlsParams::fromRawData([ + 'orderBy' => $expectedOrderBy, + ]))->willReturn(new Paginator(new ArrayAdapter())); $this->commandTester->setInputs(['n']); $this->commandTester->execute($commandArgs); diff --git a/module/CLI/test/Command/ShortUrl/ResolveUrlCommandTest.php b/module/CLI/test/Command/ShortUrl/ResolveUrlCommandTest.php index cb3e658d..7c307252 100644 --- a/module/CLI/test/Command/ShortUrl/ResolveUrlCommandTest.php +++ b/module/CLI/test/Command/ShortUrl/ResolveUrlCommandTest.php @@ -9,6 +9,7 @@ use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\CLI\Command\ShortUrl\ResolveUrlCommand; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; +use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Service\ShortUrl\ShortUrlResolverInterface; use Symfony\Component\Console\Application; use Symfony\Component\Console\Tester\CommandTester; @@ -38,8 +39,8 @@ class ResolveUrlCommandTest extends TestCase $shortCode = 'abc123'; $expectedUrl = 'http://domain.com/foo/bar'; $shortUrl = new ShortUrl($expectedUrl); - $this->urlResolver->shortCodeToShortUrl($shortCode, null)->willReturn($shortUrl) - ->shouldBeCalledOnce(); + $this->urlResolver->resolveShortUrl(new ShortUrlIdentifier($shortCode))->willReturn($shortUrl) + ->shouldBeCalledOnce(); $this->commandTester->execute(['shortCode' => $shortCode]); $output = $this->commandTester->getDisplay(); @@ -49,9 +50,11 @@ class ResolveUrlCommandTest extends TestCase /** @test */ public function incorrectShortCodeOutputsErrorMessage(): void { - $shortCode = 'abc123'; - $this->urlResolver->shortCodeToShortUrl($shortCode, null) - ->willThrow(ShortUrlNotFoundException::fromNotFoundShortCode($shortCode)) + $identifier = new ShortUrlIdentifier('abc123'); + $shortCode = $identifier->shortCode(); + + $this->urlResolver->resolveShortUrl($identifier) + ->willThrow(ShortUrlNotFoundException::fromNotFound($identifier)) ->shouldBeCalledOnce(); $this->commandTester->execute(['shortCode' => $shortCode]); diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index e1954329..9809c5dd 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -53,10 +53,14 @@ return [ Service\UrlShortener::class => [Util\UrlValidator::class, 'em', Options\UrlShortenerOptions::class], Service\VisitsTracker::class => ['em', EventDispatcherInterface::class], - Service\ShortUrlService::class => ['em'], + Service\ShortUrlService::class => ['em', Service\ShortUrl\ShortUrlResolver::class], Service\VisitService::class => ['em'], Service\Tag\TagService::class => ['em'], - Service\ShortUrl\DeleteShortUrlService::class => ['em', Options\DeleteShortUrlsOptions::class], + Service\ShortUrl\DeleteShortUrlService::class => [ + 'em', + Options\DeleteShortUrlsOptions::class, + Service\ShortUrl\ShortUrlResolver::class, + ], Service\ShortUrl\ShortUrlResolver::class => ['em'], Util\UrlValidator::class => ['httpClient'], diff --git a/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Entity.Domain.php b/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Entity.Domain.php index de7252ee..c6349b74 100644 --- a/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Entity.Domain.php +++ b/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Entity.Domain.php @@ -6,20 +6,21 @@ namespace Shlinkio\Shlink\Core; use Doctrine\DBAL\Types\Types; use Doctrine\ORM\Mapping\Builder\ClassMetadataBuilder; -use Doctrine\ORM\Mapping\ClassMetadata; // @codingStandardsIgnoreLine +use Doctrine\ORM\Mapping\ClassMetadata; -/** @var $metadata ClassMetadata */ // @codingStandardsIgnoreLine -$builder = new ClassMetadataBuilder($metadata); +return static function (ClassMetadata $metadata, array $emConfig): void { + $builder = new ClassMetadataBuilder($metadata); -$builder->setTable('domains'); + $builder->setTable(determineTableName('domains', $emConfig)); -$builder->createField('id', Types::BIGINT) - ->columnName('id') - ->makePrimaryKey() - ->generatedValue('IDENTITY') - ->option('unsigned', true) - ->build(); + $builder->createField('id', Types::BIGINT) + ->columnName('id') + ->makePrimaryKey() + ->generatedValue('IDENTITY') + ->option('unsigned', true) + ->build(); -$builder->createField('authority', Types::STRING) - ->unique() - ->build(); + $builder->createField('authority', Types::STRING) + ->unique() + ->build(); +}; diff --git a/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Entity.ShortUrl.php b/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Entity.ShortUrl.php index 0d24d555..a4aef29f 100644 --- a/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Entity.ShortUrl.php +++ b/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Entity.ShortUrl.php @@ -6,65 +6,66 @@ namespace Shlinkio\Shlink\Core; use Doctrine\DBAL\Types\Types; use Doctrine\ORM\Mapping\Builder\ClassMetadataBuilder; -use Doctrine\ORM\Mapping\ClassMetadata; // @codingStandardsIgnoreLine +use Doctrine\ORM\Mapping\ClassMetadata; use Shlinkio\Shlink\Common\Doctrine\Type\ChronosDateTimeType; -/** @var $metadata ClassMetadata */ // @codingStandardsIgnoreLine -$builder = new ClassMetadataBuilder($metadata); +return static function (ClassMetadata $metadata, array $emConfig): void { + $builder = new ClassMetadataBuilder($metadata); -$builder->setTable('short_urls') - ->setCustomRepositoryClass(Repository\ShortUrlRepository::class); + $builder->setTable(determineTableName('short_urls', $emConfig)) + ->setCustomRepositoryClass(Repository\ShortUrlRepository::class); -$builder->createField('id', Types::BIGINT) - ->columnName('id') - ->makePrimaryKey() - ->generatedValue('IDENTITY') - ->option('unsigned', true) - ->build(); + $builder->createField('id', Types::BIGINT) + ->columnName('id') + ->makePrimaryKey() + ->generatedValue('IDENTITY') + ->option('unsigned', true) + ->build(); -$builder->createField('longUrl', Types::STRING) - ->columnName('original_url') - ->length(2048) - ->build(); + $builder->createField('longUrl', Types::STRING) + ->columnName('original_url') + ->length(2048) + ->build(); -$builder->createField('shortCode', Types::STRING) - ->columnName('short_code') - ->length(255) - ->build(); + $builder->createField('shortCode', Types::STRING) + ->columnName('short_code') + ->length(255) + ->build(); -$builder->createField('dateCreated', ChronosDateTimeType::CHRONOS_DATETIME) - ->columnName('date_created') - ->build(); + $builder->createField('dateCreated', ChronosDateTimeType::CHRONOS_DATETIME) + ->columnName('date_created') + ->build(); -$builder->createField('validSince', ChronosDateTimeType::CHRONOS_DATETIME) - ->columnName('valid_since') - ->nullable() - ->build(); + $builder->createField('validSince', ChronosDateTimeType::CHRONOS_DATETIME) + ->columnName('valid_since') + ->nullable() + ->build(); -$builder->createField('validUntil', ChronosDateTimeType::CHRONOS_DATETIME) - ->columnName('valid_until') - ->nullable() - ->build(); + $builder->createField('validUntil', ChronosDateTimeType::CHRONOS_DATETIME) + ->columnName('valid_until') + ->nullable() + ->build(); -$builder->createField('maxVisits', Types::INTEGER) - ->columnName('max_visits') - ->nullable() - ->build(); + $builder->createField('maxVisits', Types::INTEGER) + ->columnName('max_visits') + ->nullable() + ->build(); -$builder->createOneToMany('visits', Entity\Visit::class) - ->mappedBy('shortUrl') - ->fetchExtraLazy() - ->build(); + $builder->createOneToMany('visits', Entity\Visit::class) + ->mappedBy('shortUrl') + ->fetchExtraLazy() + ->build(); -$builder->createManyToMany('tags', Entity\Tag::class) - ->setJoinTable('short_urls_in_tags') - ->addInverseJoinColumn('tag_id', 'id', true, false, 'CASCADE') - ->addJoinColumn('short_url_id', 'id', true, false, 'CASCADE') - ->build(); + $builder->createManyToMany('tags', Entity\Tag::class) + ->setJoinTable(determineTableName('short_urls_in_tags', $emConfig)) + ->addInverseJoinColumn('tag_id', 'id', true, false, 'CASCADE') + ->addJoinColumn('short_url_id', 'id', true, false, 'CASCADE') + ->build(); -$builder->createManyToOne('domain', Entity\Domain::class) - ->addJoinColumn('domain_id', 'id', true, false, 'RESTRICT') - ->cascadePersist() - ->build(); + $builder->createManyToOne('domain', Entity\Domain::class) + ->addJoinColumn('domain_id', 'id', true, false, 'RESTRICT') + ->cascadePersist() + ->build(); -$builder->addUniqueConstraint(['short_code', 'domain_id'], 'unique_short_code_plus_domain'); + $builder->addUniqueConstraint(['short_code', 'domain_id'], 'unique_short_code_plus_domain'); +}; diff --git a/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Entity.Tag.php b/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Entity.Tag.php index 09a98151..214396bd 100644 --- a/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Entity.Tag.php +++ b/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Entity.Tag.php @@ -6,21 +6,22 @@ namespace Shlinkio\Shlink\Core; use Doctrine\DBAL\Types\Types; use Doctrine\ORM\Mapping\Builder\ClassMetadataBuilder; -use Doctrine\ORM\Mapping\ClassMetadata; // @codingStandardsIgnoreLine +use Doctrine\ORM\Mapping\ClassMetadata; -/** @var $metadata ClassMetadata */ // @codingStandardsIgnoreLine -$builder = new ClassMetadataBuilder($metadata); +return static function (ClassMetadata $metadata, array $emConfig): void { + $builder = new ClassMetadataBuilder($metadata); -$builder->setTable('tags') - ->setCustomRepositoryClass(Repository\TagRepository::class); + $builder->setTable(determineTableName('tags', $emConfig)) + ->setCustomRepositoryClass(Repository\TagRepository::class); -$builder->createField('id', Types::BIGINT) - ->columnName('id') - ->makePrimaryKey() - ->generatedValue('IDENTITY') - ->option('unsigned', true) - ->build(); + $builder->createField('id', Types::BIGINT) + ->columnName('id') + ->makePrimaryKey() + ->generatedValue('IDENTITY') + ->option('unsigned', true) + ->build(); -$builder->createField('name', Types::STRING) - ->unique() - ->build(); + $builder->createField('name', Types::STRING) + ->unique() + ->build(); +}; diff --git a/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Entity.Visit.php b/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Entity.Visit.php index 6770f9d3..803b9790 100644 --- a/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Entity.Visit.php +++ b/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Entity.Visit.php @@ -6,49 +6,50 @@ namespace Shlinkio\Shlink\Core; use Doctrine\DBAL\Types\Types; use Doctrine\ORM\Mapping\Builder\ClassMetadataBuilder; -use Doctrine\ORM\Mapping\ClassMetadata; // @codingStandardsIgnoreLine +use Doctrine\ORM\Mapping\ClassMetadata; use Shlinkio\Shlink\Common\Doctrine\Type\ChronosDateTimeType; use Shlinkio\Shlink\Core\Model\Visitor; -/** @var $metadata ClassMetadata */ // @codingStandardsIgnoreLine -$builder = new ClassMetadataBuilder($metadata); +return static function (ClassMetadata $metadata, array $emConfig): void { + $builder = new ClassMetadataBuilder($metadata); -$builder->setTable('visits') - ->setCustomRepositoryClass(Repository\VisitRepository::class); + $builder->setTable(determineTableName('visits', $emConfig)) + ->setCustomRepositoryClass(Repository\VisitRepository::class); -$builder->createField('id', Types::BIGINT) - ->columnName('id') - ->makePrimaryKey() - ->generatedValue('IDENTITY') - ->option('unsigned', true) - ->build(); + $builder->createField('id', Types::BIGINT) + ->columnName('id') + ->makePrimaryKey() + ->generatedValue('IDENTITY') + ->option('unsigned', true) + ->build(); -$builder->createField('referer', Types::STRING) - ->nullable() - ->length(Visitor::REFERER_MAX_LENGTH) - ->build(); + $builder->createField('referer', Types::STRING) + ->nullable() + ->length(Visitor::REFERER_MAX_LENGTH) + ->build(); -$builder->createField('date', ChronosDateTimeType::CHRONOS_DATETIME) - ->columnName('`date`') - ->build(); + $builder->createField('date', ChronosDateTimeType::CHRONOS_DATETIME) + ->columnName('`date`') + ->build(); -$builder->createField('remoteAddr', Types::STRING) - ->columnName('remote_addr') - ->length(Visitor::REMOTE_ADDRESS_MAX_LENGTH) - ->nullable() - ->build(); + $builder->createField('remoteAddr', Types::STRING) + ->columnName('remote_addr') + ->length(Visitor::REMOTE_ADDRESS_MAX_LENGTH) + ->nullable() + ->build(); -$builder->createField('userAgent', Types::STRING) - ->columnName('user_agent') - ->length(Visitor::USER_AGENT_MAX_LENGTH) - ->nullable() - ->build(); + $builder->createField('userAgent', Types::STRING) + ->columnName('user_agent') + ->length(Visitor::USER_AGENT_MAX_LENGTH) + ->nullable() + ->build(); -$builder->createManyToOne('shortUrl', Entity\ShortUrl::class) - ->addJoinColumn('short_url_id', 'id', false, false, 'CASCADE') - ->build(); + $builder->createManyToOne('shortUrl', Entity\ShortUrl::class) + ->addJoinColumn('short_url_id', 'id', false, false, 'CASCADE') + ->build(); -$builder->createManyToOne('visitLocation', Entity\VisitLocation::class) - ->addJoinColumn('visit_location_id', 'id', true, false, 'Set NULL') - ->cascadePersist() - ->build(); + $builder->createManyToOne('visitLocation', Entity\VisitLocation::class) + ->addJoinColumn('visit_location_id', 'id', true, false, 'Set NULL') + ->cascadePersist() + ->build(); +}; diff --git a/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Entity.VisitLocation.php b/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Entity.VisitLocation.php index 117c2acc..fde00abc 100644 --- a/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Entity.VisitLocation.php +++ b/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Entity.VisitLocation.php @@ -6,41 +6,42 @@ namespace Shlinkio\Shlink\Core; use Doctrine\DBAL\Types\Types; use Doctrine\ORM\Mapping\Builder\ClassMetadataBuilder; -use Doctrine\ORM\Mapping\ClassMetadata; // @codingStandardsIgnoreLine +use Doctrine\ORM\Mapping\ClassMetadata; -/** @var $metadata ClassMetadata */ // @codingStandardsIgnoreLine -$builder = new ClassMetadataBuilder($metadata); +return static function (ClassMetadata $metadata, array $emConfig): void { + $builder = new ClassMetadataBuilder($metadata); -$builder->setTable('visit_locations'); + $builder->setTable(determineTableName('visit_locations', $emConfig)); -$builder->createField('id', Types::BIGINT) - ->columnName('id') - ->makePrimaryKey() - ->generatedValue('IDENTITY') - ->option('unsigned', true) - ->build(); - -$columns = [ - 'country_code' => 'countryCode', - 'country_name' => 'countryName', - 'region_name' => 'regionName', - 'city_name' => 'cityName', - 'timezone' => 'timezone', -]; - -foreach ($columns as $columnName => $fieldName) { - $builder->createField($fieldName, Types::STRING) - ->columnName($columnName) - ->nullable() + $builder->createField('id', Types::BIGINT) + ->columnName('id') + ->makePrimaryKey() + ->generatedValue('IDENTITY') + ->option('unsigned', true) ->build(); -} -$builder->createField('latitude', Types::FLOAT) - ->columnName('lat') - ->nullable(false) - ->build(); + $columns = [ + 'country_code' => 'countryCode', + 'country_name' => 'countryName', + 'region_name' => 'regionName', + 'city_name' => 'cityName', + 'timezone' => 'timezone', + ]; -$builder->createField('longitude', Types::FLOAT) - ->columnName('lon') - ->nullable(false) - ->build(); + foreach ($columns as $columnName => $fieldName) { + $builder->createField($fieldName, Types::STRING) + ->columnName($columnName) + ->nullable() + ->build(); + } + + $builder->createField('latitude', Types::FLOAT) + ->columnName('lat') + ->nullable(false) + ->build(); + + $builder->createField('longitude', Types::FLOAT) + ->columnName('lon') + ->nullable(false) + ->build(); +}; diff --git a/module/Core/functions/functions.php b/module/Core/functions/functions.php index 92d40fe1..7ab5ebbb 100644 --- a/module/Core/functions/functions.php +++ b/module/Core/functions/functions.php @@ -4,8 +4,12 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core; +use Cake\Chronos\Chronos; +use DateTimeInterface; use PUGX\Shortid\Factory as ShortIdFactory; +use function sprintf; + function generateRandomShortCode(int $length = 5): string { static $shortIdFactory; @@ -16,3 +20,36 @@ function generateRandomShortCode(int $length = 5): string $alphabet = '0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ'; return $shortIdFactory->generate($length, $alphabet)->serialize(); } + +function parseDateFromQuery(array $query, string $dateName): ?Chronos +{ + return ! isset($query[$dateName]) || empty($query[$dateName]) ? null : Chronos::parse($query[$dateName]); +} + +/** + * @param string|DateTimeInterface|Chronos|null $date + */ +function parseDateField($date): ?Chronos +{ + if ($date === null || $date instanceof Chronos) { + return $date; + } + + if ($date instanceof DateTimeInterface) { + return Chronos::instance($date); + } + + return Chronos::parse($date); +} + +function determineTableName(string $tableName, array $emConfig = []): string +{ + $schema = $emConfig['connection']['schema'] ?? null; +// $tablePrefix = $emConfig['connection']['table_prefix'] ?? null; // TODO + + if ($schema === null) { + return $tableName; + } + + return sprintf('%s.%s', $schema, $tableName); +} diff --git a/module/Core/src/Action/AbstractTrackingAction.php b/module/Core/src/Action/AbstractTrackingAction.php index 4e45d9cd..436810e6 100644 --- a/module/Core/src/Action/AbstractTrackingAction.php +++ b/module/Core/src/Action/AbstractTrackingAction.php @@ -13,6 +13,7 @@ use Psr\Log\LoggerInterface; use Psr\Log\NullLogger; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; +use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Model\Visitor; use Shlinkio\Shlink\Core\Options\AppOptions; use Shlinkio\Shlink\Core\Service\ShortUrl\ShortUrlResolverInterface; @@ -44,17 +45,16 @@ abstract class AbstractTrackingAction implements MiddlewareInterface public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface { - $shortCode = $request->getAttribute('shortCode', ''); - $domain = $request->getUri()->getAuthority(); + $identifier = ShortUrlIdentifier::fromRedirectRequest($request); $query = $request->getQueryParams(); $disableTrackParam = $this->appOptions->getDisableTrackParam(); try { - $url = $this->urlResolver->shortCodeToEnabledShortUrl($shortCode, $domain); + $url = $this->urlResolver->resolveEnabledShortUrl($identifier); // Track visit to this short code if ($disableTrackParam === null || ! array_key_exists($disableTrackParam, $query)) { - $this->visitTracker->track($shortCode, Visitor::fromRequest($request)); + $this->visitTracker->track($url, Visitor::fromRequest($request)); } return $this->createSuccessResp($this->buildUrlToRedirectTo($url, $query, $disableTrackParam)); diff --git a/module/Core/src/Action/QrCodeAction.php b/module/Core/src/Action/QrCodeAction.php index c302a58d..7a07f2a1 100644 --- a/module/Core/src/Action/QrCodeAction.php +++ b/module/Core/src/Action/QrCodeAction.php @@ -14,6 +14,7 @@ use Psr\Log\LoggerInterface; use Psr\Log\NullLogger; use Shlinkio\Shlink\Common\Response\QrCodeResponse; use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; +use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Service\ShortUrl\ShortUrlResolverInterface; class QrCodeAction implements MiddlewareInterface @@ -38,18 +39,16 @@ class QrCodeAction implements MiddlewareInterface public function process(Request $request, RequestHandlerInterface $handler): Response { - // Make sure the short URL exists for this short code - $shortCode = $request->getAttribute('shortCode'); - $domain = $request->getUri()->getAuthority(); + $identifier = ShortUrlIdentifier::fromRedirectRequest($request); try { - $this->urlResolver->shortCodeToEnabledShortUrl($shortCode, $domain); + $this->urlResolver->resolveEnabledShortUrl($identifier); } catch (ShortUrlNotFoundException $e) { $this->logger->warning('An error occurred while creating QR code. {e}', ['e' => $e]); return $handler->handle($request); } - $path = $this->router->generateUri(RedirectAction::class, ['shortCode' => $shortCode]); + $path = $this->router->generateUri(RedirectAction::class, ['shortCode' => $identifier->shortCode()]); $size = $this->getSizeParam($request); $qrCode = new QrCode((string) $request->getUri()->withPath($path)->withQuery('')); diff --git a/module/Core/src/Entity/Domain.php b/module/Core/src/Entity/Domain.php index 924b50e5..f836f7ed 100644 --- a/module/Core/src/Entity/Domain.php +++ b/module/Core/src/Entity/Domain.php @@ -4,9 +4,10 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Entity; +use JsonSerializable; use Shlinkio\Shlink\Common\Entity\AbstractEntity; -class Domain extends AbstractEntity +class Domain extends AbstractEntity implements JsonSerializable { private string $authority; @@ -19,4 +20,9 @@ class Domain extends AbstractEntity { return $this->authority; } + + public function jsonSerialize(): string + { + return $this->getAuthority(); + } } diff --git a/module/Core/src/Entity/ShortUrl.php b/module/Core/src/Entity/ShortUrl.php index e260896d..98d6a146 100644 --- a/module/Core/src/Entity/ShortUrl.php +++ b/module/Core/src/Entity/ShortUrl.php @@ -69,6 +69,11 @@ class ShortUrl extends AbstractEntity return $this->dateCreated; } + public function getDomain(): ?Domain + { + return $this->domain; + } + /** * @return Collection|Tag[] */ diff --git a/module/Core/src/Exception/ShortUrlNotFoundException.php b/module/Core/src/Exception/ShortUrlNotFoundException.php index e68e55ed..0ae29da5 100644 --- a/module/Core/src/Exception/ShortUrlNotFoundException.php +++ b/module/Core/src/Exception/ShortUrlNotFoundException.php @@ -7,6 +7,7 @@ namespace Shlinkio\Shlink\Core\Exception; use Fig\Http\Message\StatusCodeInterface; use Mezzio\ProblemDetails\Exception\CommonProblemDetailsExceptionTrait; use Mezzio\ProblemDetails\Exception\ProblemDetailsExceptionInterface; +use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use function sprintf; @@ -17,8 +18,10 @@ class ShortUrlNotFoundException extends DomainException implements ProblemDetail private const TITLE = 'Short URL not found'; private const TYPE = 'INVALID_SHORTCODE'; - public static function fromNotFoundShortCode(string $shortCode, ?string $domain = null): self + public static function fromNotFound(ShortUrlIdentifier $identifier): self { + $shortCode = $identifier->shortCode(); + $domain = $identifier->domain(); $suffix = $domain === null ? '' : sprintf(' for domain "%s"', $domain); $e = new self(sprintf('No URL found with short code "%s"%s', $shortCode, $suffix)); diff --git a/module/Core/src/Model/ShortUrlIdentifier.php b/module/Core/src/Model/ShortUrlIdentifier.php new file mode 100644 index 00000000..4a74ba07 --- /dev/null +++ b/module/Core/src/Model/ShortUrlIdentifier.php @@ -0,0 +1,54 @@ +shortCode = $shortCode; + $this->domain = $domain; + } + + public static function fromApiRequest(ServerRequestInterface $request): self + { + $shortCode = $request->getAttribute('shortCode', ''); + $domain = $request->getQueryParams()['domain'] ?? null; + + return new self($shortCode, $domain); + } + + public static function fromRedirectRequest(ServerRequestInterface $request): self + { + $shortCode = $request->getAttribute('shortCode', ''); + $domain = $request->getUri()->getAuthority(); + + return new self($shortCode, $domain); + } + + public static function fromCli(InputInterface $input): self + { + $shortCode = $input->getArguments()['shortCode'] ?? ''; + $domain = $input->getOptions()['domain'] ?? null; + + return new self($shortCode, $domain); + } + + public function shortCode(): string + { + return $this->shortCode; + } + + public function domain(): ?string + { + return $this->domain; + } +} diff --git a/module/Core/src/Model/ShortUrlMeta.php b/module/Core/src/Model/ShortUrlMeta.php index f0c487b7..27c8e624 100644 --- a/module/Core/src/Model/ShortUrlMeta.php +++ b/module/Core/src/Model/ShortUrlMeta.php @@ -5,11 +5,11 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Model; use Cake\Chronos\Chronos; -use DateTimeInterface; use Shlinkio\Shlink\Core\Exception\ValidationException; use Shlinkio\Shlink\Core\Validation\ShortUrlMetaInputFilter; use function array_key_exists; +use function Shlinkio\Shlink\Core\parseDateField; final class ShortUrlMeta { @@ -34,30 +34,28 @@ final class ShortUrlMeta } /** - * @param array $data * @throws ValidationException */ public static function fromRawData(array $data): self { $instance = new self(); - $instance->validate($data); + $instance->validateAndInit($data); return $instance; } /** - * @param array $data * @throws ValidationException */ - private function validate(array $data): void + private function validateAndInit(array $data): void { $inputFilter = new ShortUrlMetaInputFilter($data); if (! $inputFilter->isValid()) { throw ValidationException::fromInputFilter($inputFilter); } - $this->validSince = $this->parseDateField($inputFilter->getValue(ShortUrlMetaInputFilter::VALID_SINCE)); + $this->validSince = parseDateField($inputFilter->getValue(ShortUrlMetaInputFilter::VALID_SINCE)); $this->validSincePropWasProvided = array_key_exists(ShortUrlMetaInputFilter::VALID_SINCE, $data); - $this->validUntil = $this->parseDateField($inputFilter->getValue(ShortUrlMetaInputFilter::VALID_UNTIL)); + $this->validUntil = parseDateField($inputFilter->getValue(ShortUrlMetaInputFilter::VALID_UNTIL)); $this->validUntilPropWasProvided = array_key_exists(ShortUrlMetaInputFilter::VALID_UNTIL, $data); $this->customSlug = $inputFilter->getValue(ShortUrlMetaInputFilter::CUSTOM_SLUG); $maxVisits = $inputFilter->getValue(ShortUrlMetaInputFilter::MAX_VISITS); @@ -67,22 +65,6 @@ final class ShortUrlMeta $this->domain = $inputFilter->getValue(ShortUrlMetaInputFilter::DOMAIN); } - /** - * @param string|DateTimeInterface|Chronos|null $date - */ - private function parseDateField($date): ?Chronos - { - if ($date === null || $date instanceof Chronos) { - return $date; - } - - if ($date instanceof DateTimeInterface) { - return Chronos::instance($date); - } - - return Chronos::parse($date); - } - public function getValidSince(): ?Chronos { return $this->validSince; diff --git a/module/Core/src/Model/ShortUrlsOrdering.php b/module/Core/src/Model/ShortUrlsOrdering.php new file mode 100644 index 00000000..00c30a54 --- /dev/null +++ b/module/Core/src/Model/ShortUrlsOrdering.php @@ -0,0 +1,68 @@ +validateAndInit($query); + + return $instance; + } + + /** + * @throws ValidationException + */ + private function validateAndInit(array $data): void + { + /** @var string|array|null $orderBy */ + $orderBy = $data[self::ORDER_BY] ?? null; + if ($orderBy === null) { + return; + } + + $isArray = is_array($orderBy); + if (! $isArray && $orderBy !== null && ! is_string($orderBy)) { + throw ValidationException::fromArray([ + 'orderBy' => '"Order by" must be an array, string or null', + ]); + } + + $this->orderField = $isArray ? key($orderBy) : $orderBy; + $this->orderDirection = $isArray ? $orderBy[$this->orderField] : self::DEFAULT_ORDER_DIRECTION; + } + + public function orderField(): ?string + { + return $this->orderField; + } + + public function orderDirection(): string + { + return $this->orderDirection; + } + + public function hasOrderField(): bool + { + return $this->orderField !== null; + } +} diff --git a/module/Core/src/Model/ShortUrlsParams.php b/module/Core/src/Model/ShortUrlsParams.php new file mode 100644 index 00000000..cbd74bec --- /dev/null +++ b/module/Core/src/Model/ShortUrlsParams.php @@ -0,0 +1,85 @@ +validateAndInit($query); + + return $instance; + } + + /** + * @throws ValidationException + */ + private function validateAndInit(array $query): void + { + $inputFilter = new ShortUrlsParamsInputFilter($query); + if (! $inputFilter->isValid()) { + throw ValidationException::fromInputFilter($inputFilter); + } + + $this->page = (int) ($inputFilter->getValue(ShortUrlsParamsInputFilter::PAGE) ?? 1); + $this->searchTerm = $inputFilter->getValue(ShortUrlsParamsInputFilter::SEARCH_TERM); + $this->tags = (array) $inputFilter->getValue(ShortUrlsParamsInputFilter::TAGS); + $this->dateRange = new DateRange( + parseDateField($inputFilter->getValue(ShortUrlsParamsInputFilter::START_DATE)), + parseDateField($inputFilter->getValue(ShortUrlsParamsInputFilter::END_DATE)), + ); + $this->orderBy = ShortUrlsOrdering::fromRawData($query); + } + + public function page(): int + { + return $this->page; + } + + public function searchTerm(): ?string + { + return $this->searchTerm; + } + + public function tags(): array + { + return $this->tags; + } + + public function orderBy(): ShortUrlsOrdering + { + return $this->orderBy; + } + + public function dateRange(): ?DateRange + { + return $this->dateRange; + } +} diff --git a/module/Core/src/Model/VisitsParams.php b/module/Core/src/Model/VisitsParams.php index 98fcbe82..041aed9f 100644 --- a/module/Core/src/Model/VisitsParams.php +++ b/module/Core/src/Model/VisitsParams.php @@ -4,9 +4,10 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Model; -use Cake\Chronos\Chronos; use Shlinkio\Shlink\Common\Util\DateRange; +use function Shlinkio\Shlink\Core\parseDateFromQuery; + final class VisitsParams { private const FIRST_PAGE = 1; @@ -34,21 +35,13 @@ final class VisitsParams public static function fromRawData(array $query): self { - $startDate = self::getDateQueryParam($query, 'startDate'); - $endDate = self::getDateQueryParam($query, 'endDate'); - return new self( - new DateRange($startDate, $endDate), + new DateRange(parseDateFromQuery($query, 'startDate'), parseDateFromQuery($query, 'endDate')), (int) ($query['page'] ?? 1), isset($query['itemsPerPage']) ? (int) $query['itemsPerPage'] : null, ); } - private static function getDateQueryParam(array $query, string $key): ?Chronos - { - return ! isset($query[$key]) || empty($query[$key]) ? null : Chronos::parse($query[$key]); - } - public function getDateRange(): DateRange { return $this->dateRange; diff --git a/module/Core/src/Paginator/Adapter/ShortUrlRepositoryAdapter.php b/module/Core/src/Paginator/Adapter/ShortUrlRepositoryAdapter.php index d15d925c..a4cf3190 100644 --- a/module/Core/src/Paginator/Adapter/ShortUrlRepositoryAdapter.php +++ b/module/Core/src/Paginator/Adapter/ShortUrlRepositoryAdapter.php @@ -5,38 +5,20 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Paginator\Adapter; use Laminas\Paginator\Adapter\AdapterInterface; -use Shlinkio\Shlink\Common\Util\DateRange; +use Shlinkio\Shlink\Core\Model\ShortUrlsParams; use Shlinkio\Shlink\Core\Repository\ShortUrlRepositoryInterface; -use function strip_tags; -use function trim; - class ShortUrlRepositoryAdapter implements AdapterInterface { public const ITEMS_PER_PAGE = 10; private ShortUrlRepositoryInterface $repository; - private ?string $searchTerm; - /** @var null|array|string */ - private $orderBy; - private array $tags; - private ?DateRange $dateRange; + private ShortUrlsParams $params; - /** - * @param string|array|null $orderBy - */ - public function __construct( - ShortUrlRepositoryInterface $repository, - ?string $searchTerm = null, - array $tags = [], - $orderBy = null, - ?DateRange $dateRange = null - ) { + public function __construct(ShortUrlRepositoryInterface $repository, ShortUrlsParams $params) + { $this->repository = $repository; - $this->searchTerm = $searchTerm !== null ? trim(strip_tags($searchTerm)) : null; - $this->orderBy = $orderBy; - $this->tags = $tags; - $this->dateRange = $dateRange; + $this->params = $params; } /** @@ -50,10 +32,10 @@ class ShortUrlRepositoryAdapter implements AdapterInterface return $this->repository->findList( $itemCountPerPage, $offset, - $this->searchTerm, - $this->tags, - $this->orderBy, - $this->dateRange, + $this->params->searchTerm(), + $this->params->tags(), + $this->params->orderBy(), + $this->params->dateRange(), ); } @@ -68,6 +50,10 @@ class ShortUrlRepositoryAdapter implements AdapterInterface */ public function count(): int { - return $this->repository->countList($this->searchTerm, $this->tags, $this->dateRange); + return $this->repository->countList( + $this->params->searchTerm(), + $this->params->tags(), + $this->params->dateRange(), + ); } } diff --git a/module/Core/src/Paginator/Adapter/VisitsPaginatorAdapter.php b/module/Core/src/Paginator/Adapter/VisitsPaginatorAdapter.php index 19baff73..247ea93e 100644 --- a/module/Core/src/Paginator/Adapter/VisitsPaginatorAdapter.php +++ b/module/Core/src/Paginator/Adapter/VisitsPaginatorAdapter.php @@ -5,26 +5,31 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Paginator\Adapter; use Laminas\Paginator\Adapter\AdapterInterface; +use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Model\VisitsParams; use Shlinkio\Shlink\Core\Repository\VisitRepositoryInterface; class VisitsPaginatorAdapter implements AdapterInterface { private VisitRepositoryInterface $visitRepository; - private string $shortCode; + private ShortUrlIdentifier $identifier; private VisitsParams $params; - public function __construct(VisitRepositoryInterface $visitRepository, string $shortCode, VisitsParams $params) - { + public function __construct( + VisitRepositoryInterface $visitRepository, + ShortUrlIdentifier $identifier, + VisitsParams $params + ) { $this->visitRepository = $visitRepository; - $this->shortCode = $shortCode; $this->params = $params; + $this->identifier = $identifier; } public function getItems($offset, $itemCountPerPage): array // phpcs:ignore { return $this->visitRepository->findVisitsByShortCode( - $this->shortCode, + $this->identifier->shortCode(), + $this->identifier->domain(), $this->params->getDateRange(), $itemCountPerPage, $offset, @@ -33,6 +38,10 @@ class VisitsPaginatorAdapter implements AdapterInterface public function count(): int { - return $this->visitRepository->countVisitsByShortCode($this->shortCode, $this->params->getDateRange()); + return $this->visitRepository->countVisitsByShortCode( + $this->identifier->shortCode(), + $this->identifier->domain(), + $this->params->getDateRange(), + ); } } diff --git a/module/Core/src/Repository/ShortUrlRepository.php b/module/Core/src/Repository/ShortUrlRepository.php index a334a838..31fe1385 100644 --- a/module/Core/src/Repository/ShortUrlRepository.php +++ b/module/Core/src/Repository/ShortUrlRepository.php @@ -8,18 +8,16 @@ use Doctrine\ORM\EntityRepository; use Doctrine\ORM\QueryBuilder; use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\Entity\ShortUrl; +use Shlinkio\Shlink\Core\Model\ShortUrlsOrdering; use function array_column; use function array_key_exists; use function Functional\contains; -use function is_array; -use function key; class ShortUrlRepository extends EntityRepository implements ShortUrlRepositoryInterface { /** * @param string[] $tags - * @param string|array|null $orderBy * @return ShortUrl[] */ public function findList( @@ -27,7 +25,7 @@ class ShortUrlRepository extends EntityRepository implements ShortUrlRepositoryI ?int $offset = null, ?string $searchTerm = null, array $tags = [], - $orderBy = null, + ?ShortUrlsOrdering $orderBy = null, ?DateRange $dateRange = null ): array { $qb = $this->createListQueryBuilder($searchTerm, $tags, $dateRange); @@ -42,7 +40,7 @@ class ShortUrlRepository extends EntityRepository implements ShortUrlRepositoryI } // In case the ordering has been specified, the query could be more complex. Process it - if ($orderBy !== null) { + if ($orderBy !== null && $orderBy->hasOrderField()) { return $this->processOrderByForList($qb, $orderBy); } @@ -51,14 +49,10 @@ class ShortUrlRepository extends EntityRepository implements ShortUrlRepositoryI return $qb->getQuery()->getResult(); } - /** - * @param string|array|null $orderBy - */ - private function processOrderByForList(QueryBuilder $qb, $orderBy): array + private function processOrderByForList(QueryBuilder $qb, ShortUrlsOrdering $orderBy): array { - $isArray = is_array($orderBy); - $fieldName = $isArray ? key($orderBy) : $orderBy; - $order = $isArray ? $orderBy[$fieldName] : 'ASC'; + $fieldName = $orderBy->orderField(); + $order = $orderBy->orderDirection(); if (contains(['visits', 'visitsCount', 'visitCount'], $fieldName)) { $qb->addSelect('COUNT(DISTINCT v) AS totalVisits') @@ -96,8 +90,8 @@ class ShortUrlRepository extends EntityRepository implements ShortUrlRepositoryI ?DateRange $dateRange = null ): QueryBuilder { $qb = $this->getEntityManager()->createQueryBuilder(); - $qb->from(ShortUrl::class, 's'); - $qb->where('1=1'); + $qb->from(ShortUrl::class, 's') + ->where('1=1'); if ($dateRange !== null && $dateRange->getStartDate() !== null) { $qb->andWhere($qb->expr()->gte('s.dateCreated', ':startDate')); @@ -116,12 +110,14 @@ class ShortUrlRepository extends EntityRepository implements ShortUrlRepositoryI } // Apply search conditions - $qb->andWhere($qb->expr()->orX( - $qb->expr()->like('s.longUrl', ':searchPattern'), - $qb->expr()->like('s.shortCode', ':searchPattern'), - $qb->expr()->like('t.name', ':searchPattern'), - )); - $qb->setParameter('searchPattern', '%' . $searchTerm . '%'); + $qb->leftJoin('s.domain', 'd') + ->andWhere($qb->expr()->orX( + $qb->expr()->like('s.longUrl', ':searchPattern'), + $qb->expr()->like('s.shortCode', ':searchPattern'), + $qb->expr()->like('t.name', ':searchPattern'), + $qb->expr()->like('d.authority', ':searchPattern'), + )) + ->setParameter('searchPattern', '%' . $searchTerm . '%'); } // Filter by tags if provided @@ -133,7 +129,7 @@ class ShortUrlRepository extends EntityRepository implements ShortUrlRepositoryI return $qb; } - public function findOneByShortCode(string $shortCode, ?string $domain = null): ?ShortUrl + public function findOneWithDomainFallback(string $shortCode, ?string $domain = null): ?ShortUrl { // When ordering DESC, Postgres puts nulls at the beginning while the rest of supported DB engines put them at // the bottom @@ -165,14 +161,30 @@ DQL; return $query->getOneOrNullResult(); } + public function findOne(string $shortCode, ?string $domain = null): ?ShortUrl + { + $qb = $this->createFindOneQueryBuilder($shortCode, $domain); + $qb->select('s'); + + return $qb->getQuery()->getOneOrNullResult(); + } + public function shortCodeIsInUse(string $slug, ?string $domain = null): bool + { + $qb = $this->createFindOneQueryBuilder($slug, $domain); + $qb->select('COUNT(DISTINCT s.id)'); + + return ((int) $qb->getQuery()->getSingleScalarResult()) > 0; + } + + private function createFindOneQueryBuilder(string $slug, ?string $domain = null): QueryBuilder { $qb = $this->getEntityManager()->createQueryBuilder(); - $qb->select('COUNT(DISTINCT s.id)') - ->from(ShortUrl::class, 's') + $qb->from(ShortUrl::class, 's') ->where($qb->expr()->isNotNull('s.shortCode')) ->andWhere($qb->expr()->eq('s.shortCode', ':slug')) - ->setParameter('slug', $slug); + ->setParameter('slug', $slug) + ->setMaxResults(1); if ($domain !== null) { $qb->join('s.domain', 'd') @@ -182,7 +194,6 @@ DQL; $qb->andWhere($qb->expr()->isNull('s.domain')); } - $result = (int) $qb->getQuery()->getSingleScalarResult(); - return $result > 0; + return $qb; } } diff --git a/module/Core/src/Repository/ShortUrlRepositoryInterface.php b/module/Core/src/Repository/ShortUrlRepositoryInterface.php index 8695021a..065198b4 100644 --- a/module/Core/src/Repository/ShortUrlRepositoryInterface.php +++ b/module/Core/src/Repository/ShortUrlRepositoryInterface.php @@ -7,24 +7,24 @@ namespace Shlinkio\Shlink\Core\Repository; use Doctrine\Persistence\ObjectRepository; use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\Entity\ShortUrl; +use Shlinkio\Shlink\Core\Model\ShortUrlsOrdering; interface ShortUrlRepositoryInterface extends ObjectRepository { - /** - * @param string|array|null $orderBy - */ public function findList( ?int $limit = null, ?int $offset = null, ?string $searchTerm = null, array $tags = [], - $orderBy = null, + ?ShortUrlsOrdering $orderBy = null, ?DateRange $dateRange = null ): array; public function countList(?string $searchTerm = null, array $tags = [], ?DateRange $dateRange = null): int; - public function findOneByShortCode(string $shortCode, ?string $domain = null): ?ShortUrl; + public function findOneWithDomainFallback(string $shortCode, ?string $domain = null): ?ShortUrl; + + public function findOne(string $shortCode, ?string $domain = null): ?ShortUrl; public function shortCodeIsInUse(string $slug, ?string $domain): bool; } diff --git a/module/Core/src/Repository/VisitRepository.php b/module/Core/src/Repository/VisitRepository.php index 0e48e0a1..454323ef 100644 --- a/module/Core/src/Repository/VisitRepository.php +++ b/module/Core/src/Repository/VisitRepository.php @@ -46,11 +46,12 @@ DQL; */ public function findVisitsByShortCode( string $shortCode, + ?string $domain = null, ?DateRange $dateRange = null, ?int $limit = null, ?int $offset = null ): array { - $qb = $this->createVisitsByShortCodeQueryBuilder($shortCode, $dateRange); + $qb = $this->createVisitsByShortCodeQueryBuilder($shortCode, $domain, $dateRange); $qb->select('v') ->orderBy('v.date', 'DESC'); @@ -64,22 +65,34 @@ DQL; return $qb->getQuery()->getResult(); } - public function countVisitsByShortCode(string $shortCode, ?DateRange $dateRange = null): int + public function countVisitsByShortCode(string $shortCode, ?string $domain = null, ?DateRange $dateRange = null): int { - $qb = $this->createVisitsByShortCodeQueryBuilder($shortCode, $dateRange); + $qb = $this->createVisitsByShortCodeQueryBuilder($shortCode, $domain, $dateRange); $qb->select('COUNT(DISTINCT v.id)'); return (int) $qb->getQuery()->getSingleScalarResult(); } - private function createVisitsByShortCodeQueryBuilder(string $shortCode, ?DateRange $dateRange = null): QueryBuilder - { + private function createVisitsByShortCodeQueryBuilder( + string $shortCode, + ?string $domain, + ?DateRange $dateRange + ): QueryBuilder { $qb = $this->getEntityManager()->createQueryBuilder(); $qb->from(Visit::class, 'v') ->join('v.shortUrl', 'su') ->where($qb->expr()->eq('su.shortCode', ':shortCode')) ->setParameter('shortCode', $shortCode); + // Apply domain filtering + if ($domain !== null) { + $qb->join('su.domain', 'd') + ->andWhere($qb->expr()->eq('d.authority', ':domain')) + ->setParameter('domain', $domain); + } else { + $qb->andWhere($qb->expr()->isNull('su.domain')); + } + // Apply date range filtering if ($dateRange !== null && $dateRange->getStartDate() !== null) { $qb->andWhere($qb->expr()->gte('v.date', ':startDate')) diff --git a/module/Core/src/Repository/VisitRepositoryInterface.php b/module/Core/src/Repository/VisitRepositoryInterface.php index e70c989e..0d0b66d0 100644 --- a/module/Core/src/Repository/VisitRepositoryInterface.php +++ b/module/Core/src/Repository/VisitRepositoryInterface.php @@ -28,10 +28,15 @@ interface VisitRepositoryInterface extends ObjectRepository */ public function findVisitsByShortCode( string $shortCode, + ?string $domain = null, ?DateRange $dateRange = null, ?int $limit = null, ?int $offset = null ): array; - public function countVisitsByShortCode(string $shortCode, ?DateRange $dateRange = null): int; + public function countVisitsByShortCode( + string $shortCode, + ?string $domain = null, + ?DateRange $dateRange = null + ): int; } diff --git a/module/Core/src/Service/ShortUrl/DeleteShortUrlService.php b/module/Core/src/Service/ShortUrl/DeleteShortUrlService.php index c9624bf3..35a540da 100644 --- a/module/Core/src/Service/ShortUrl/DeleteShortUrlService.php +++ b/module/Core/src/Service/ShortUrl/DeleteShortUrlService.php @@ -7,28 +7,32 @@ namespace Shlinkio\Shlink\Core\Service\ShortUrl; use Doctrine\ORM\EntityManagerInterface; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Exception; +use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Options\DeleteShortUrlsOptions; class DeleteShortUrlService implements DeleteShortUrlServiceInterface { - use FindShortCodeTrait; - private EntityManagerInterface $em; private DeleteShortUrlsOptions $deleteShortUrlsOptions; + private ShortUrlResolverInterface $urlResolver; - public function __construct(EntityManagerInterface $em, DeleteShortUrlsOptions $deleteShortUrlsOptions) - { + public function __construct( + EntityManagerInterface $em, + DeleteShortUrlsOptions $deleteShortUrlsOptions, + ShortUrlResolverInterface $urlResolver + ) { $this->em = $em; $this->deleteShortUrlsOptions = $deleteShortUrlsOptions; + $this->urlResolver = $urlResolver; } /** * @throws Exception\ShortUrlNotFoundException * @throws Exception\DeleteShortUrlException */ - public function deleteByShortCode(string $shortCode, bool $ignoreThreshold = false): void + public function deleteByShortCode(ShortUrlIdentifier $identifier, bool $ignoreThreshold = false): void { - $shortUrl = $this->findByShortCode($this->em, $shortCode); + $shortUrl = $this->urlResolver->resolveShortUrl($identifier); if (! $ignoreThreshold && $this->isThresholdReached($shortUrl)) { throw Exception\DeleteShortUrlException::fromVisitsThreshold( $this->deleteShortUrlsOptions->getVisitsThreshold(), diff --git a/module/Core/src/Service/ShortUrl/DeleteShortUrlServiceInterface.php b/module/Core/src/Service/ShortUrl/DeleteShortUrlServiceInterface.php index b196375d..4759bf24 100644 --- a/module/Core/src/Service/ShortUrl/DeleteShortUrlServiceInterface.php +++ b/module/Core/src/Service/ShortUrl/DeleteShortUrlServiceInterface.php @@ -5,6 +5,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Service\ShortUrl; use Shlinkio\Shlink\Core\Exception; +use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; interface DeleteShortUrlServiceInterface { @@ -12,5 +13,5 @@ interface DeleteShortUrlServiceInterface * @throws Exception\ShortUrlNotFoundException * @throws Exception\DeleteShortUrlException */ - public function deleteByShortCode(string $shortCode, bool $ignoreThreshold = false): void; + public function deleteByShortCode(ShortUrlIdentifier $identifier, bool $ignoreThreshold = false): void; } diff --git a/module/Core/src/Service/ShortUrl/FindShortCodeTrait.php b/module/Core/src/Service/ShortUrl/FindShortCodeTrait.php deleted file mode 100644 index 95009704..00000000 --- a/module/Core/src/Service/ShortUrl/FindShortCodeTrait.php +++ /dev/null @@ -1,28 +0,0 @@ -getRepository(ShortUrl::class)->findOneBy([ - 'shortCode' => $shortCode, - ]); - if ($shortUrl === null) { - throw ShortUrlNotFoundException::fromNotFoundShortCode($shortCode); - } - - return $shortUrl; - } -} diff --git a/module/Core/src/Service/ShortUrl/ShortUrlResolver.php b/module/Core/src/Service/ShortUrl/ShortUrlResolver.php index 62f30c11..414a3446 100644 --- a/module/Core/src/Service/ShortUrl/ShortUrlResolver.php +++ b/module/Core/src/Service/ShortUrl/ShortUrlResolver.php @@ -7,6 +7,7 @@ namespace Shlinkio\Shlink\Core\Service\ShortUrl; use Doctrine\ORM\EntityManagerInterface; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; +use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Repository\ShortUrlRepository; class ShortUrlResolver implements ShortUrlResolverInterface @@ -21,13 +22,13 @@ class ShortUrlResolver implements ShortUrlResolverInterface /** * @throws ShortUrlNotFoundException */ - public function shortCodeToShortUrl(string $shortCode, ?string $domain = null): ShortUrl + public function resolveShortUrl(ShortUrlIdentifier $identifier): ShortUrl { /** @var ShortUrlRepository $shortUrlRepo */ $shortUrlRepo = $this->em->getRepository(ShortUrl::class); - $shortUrl = $shortUrlRepo->findOneByShortCode($shortCode, $domain); + $shortUrl = $shortUrlRepo->findOne($identifier->shortCode(), $identifier->domain()); if ($shortUrl === null) { - throw ShortUrlNotFoundException::fromNotFoundShortCode($shortCode, $domain); + throw ShortUrlNotFoundException::fromNotFound($identifier); } return $shortUrl; @@ -36,11 +37,13 @@ class ShortUrlResolver implements ShortUrlResolverInterface /** * @throws ShortUrlNotFoundException */ - public function shortCodeToEnabledShortUrl(string $shortCode, ?string $domain = null): ShortUrl + public function resolveEnabledShortUrl(ShortUrlIdentifier $identifier): ShortUrl { - $shortUrl = $this->shortCodeToShortUrl($shortCode, $domain); - if (! $shortUrl->isEnabled()) { - throw ShortUrlNotFoundException::fromNotFoundShortCode($shortCode, $domain); + /** @var ShortUrlRepository $shortUrlRepo */ + $shortUrlRepo = $this->em->getRepository(ShortUrl::class); + $shortUrl = $shortUrlRepo->findOneWithDomainFallback($identifier->shortCode(), $identifier->domain()); + if ($shortUrl === null || ! $shortUrl->isEnabled()) { + throw ShortUrlNotFoundException::fromNotFound($identifier); } return $shortUrl; diff --git a/module/Core/src/Service/ShortUrl/ShortUrlResolverInterface.php b/module/Core/src/Service/ShortUrl/ShortUrlResolverInterface.php index b00beed5..a3a7c115 100644 --- a/module/Core/src/Service/ShortUrl/ShortUrlResolverInterface.php +++ b/module/Core/src/Service/ShortUrl/ShortUrlResolverInterface.php @@ -6,16 +6,17 @@ namespace Shlinkio\Shlink\Core\Service\ShortUrl; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; +use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; interface ShortUrlResolverInterface { /** * @throws ShortUrlNotFoundException */ - public function shortCodeToShortUrl(string $shortCode, ?string $domain = null): ShortUrl; + public function resolveShortUrl(ShortUrlIdentifier $identifier): ShortUrl; /** * @throws ShortUrlNotFoundException */ - public function shortCodeToEnabledShortUrl(string $shortCode, ?string $domain = null): ShortUrl; + public function resolveEnabledShortUrl(ShortUrlIdentifier $identifier): ShortUrl; } diff --git a/module/Core/src/Service/ShortUrlService.php b/module/Core/src/Service/ShortUrlService.php index 8733cddb..e9aaf637 100644 --- a/module/Core/src/Service/ShortUrlService.php +++ b/module/Core/src/Service/ShortUrlService.php @@ -6,45 +6,39 @@ namespace Shlinkio\Shlink\Core\Service; use Doctrine\ORM; use Laminas\Paginator\Paginator; -use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; +use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Model\ShortUrlMeta; +use Shlinkio\Shlink\Core\Model\ShortUrlsParams; use Shlinkio\Shlink\Core\Paginator\Adapter\ShortUrlRepositoryAdapter; use Shlinkio\Shlink\Core\Repository\ShortUrlRepository; -use Shlinkio\Shlink\Core\Service\ShortUrl\FindShortCodeTrait; +use Shlinkio\Shlink\Core\Service\ShortUrl\ShortUrlResolverInterface; use Shlinkio\Shlink\Core\Util\TagManagerTrait; class ShortUrlService implements ShortUrlServiceInterface { - use FindShortCodeTrait; use TagManagerTrait; private ORM\EntityManagerInterface $em; + private ShortUrlResolverInterface $urlResolver; - public function __construct(ORM\EntityManagerInterface $em) + public function __construct(ORM\EntityManagerInterface $em, ShortUrlResolverInterface $urlResolver) { $this->em = $em; + $this->urlResolver = $urlResolver; } /** - * @param string[] $tags - * @param array|string|null $orderBy - * * @return ShortUrl[]|Paginator */ - public function listShortUrls( - int $page = 1, - ?string $searchQuery = null, - array $tags = [], - $orderBy = null, - ?DateRange $dateRange = null - ) { + public function listShortUrls(ShortUrlsParams $params): Paginator + { /** @var ShortUrlRepository $repo */ $repo = $this->em->getRepository(ShortUrl::class); - $paginator = new Paginator(new ShortUrlRepositoryAdapter($repo, $searchQuery, $tags, $orderBy, $dateRange)); + $paginator = new Paginator(new ShortUrlRepositoryAdapter($repo, $params)); $paginator->setItemCountPerPage(ShortUrlRepositoryAdapter::ITEMS_PER_PAGE) - ->setCurrentPageNumber($page); + ->setCurrentPageNumber($params->page()); return $paginator; } @@ -53,10 +47,11 @@ class ShortUrlService implements ShortUrlServiceInterface * @param string[] $tags * @throws ShortUrlNotFoundException */ - public function setTagsByShortCode(string $shortCode, array $tags = []): ShortUrl + public function setTagsByShortCode(ShortUrlIdentifier $identifier, array $tags = []): ShortUrl { - $shortUrl = $this->findByShortCode($this->em, $shortCode); + $shortUrl = $this->urlResolver->resolveShortUrl($identifier); $shortUrl->setTags($this->tagNamesToEntities($this->em, $tags)); + $this->em->flush(); return $shortUrl; @@ -65,9 +60,9 @@ class ShortUrlService implements ShortUrlServiceInterface /** * @throws ShortUrlNotFoundException */ - public function updateMetadataByShortCode(string $shortCode, ShortUrlMeta $shortUrlMeta): ShortUrl + public function updateMetadataByShortCode(ShortUrlIdentifier $identifier, ShortUrlMeta $shortUrlMeta): ShortUrl { - $shortUrl = $this->findByShortCode($this->em, $shortCode); + $shortUrl = $this->urlResolver->resolveShortUrl($identifier); $shortUrl->updateMeta($shortUrlMeta); $this->em->flush(); diff --git a/module/Core/src/Service/ShortUrlServiceInterface.php b/module/Core/src/Service/ShortUrlServiceInterface.php index 310ba6b3..379abc55 100644 --- a/module/Core/src/Service/ShortUrlServiceInterface.php +++ b/module/Core/src/Service/ShortUrlServiceInterface.php @@ -5,35 +5,27 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Service; use Laminas\Paginator\Paginator; -use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; +use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Model\ShortUrlMeta; +use Shlinkio\Shlink\Core\Model\ShortUrlsParams; interface ShortUrlServiceInterface { /** - * @param string[] $tags - * @param array|string|null $orderBy - * * @return ShortUrl[]|Paginator */ - public function listShortUrls( - int $page = 1, - ?string $searchQuery = null, - array $tags = [], - $orderBy = null, - ?DateRange $dateRange = null - ); + public function listShortUrls(ShortUrlsParams $params): Paginator; /** * @param string[] $tags * @throws ShortUrlNotFoundException */ - public function setTagsByShortCode(string $shortCode, array $tags = []): ShortUrl; + public function setTagsByShortCode(ShortUrlIdentifier $identifier, array $tags = []): ShortUrl; /** * @throws ShortUrlNotFoundException */ - public function updateMetadataByShortCode(string $shortCode, ShortUrlMeta $shortUrlMeta): ShortUrl; + public function updateMetadataByShortCode(ShortUrlIdentifier $identifier, ShortUrlMeta $shortUrlMeta): ShortUrl; } diff --git a/module/Core/src/Service/VisitsTracker.php b/module/Core/src/Service/VisitsTracker.php index 91c83a03..54abe319 100644 --- a/module/Core/src/Service/VisitsTracker.php +++ b/module/Core/src/Service/VisitsTracker.php @@ -11,9 +11,11 @@ use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Entity\Visit; use Shlinkio\Shlink\Core\EventDispatcher\ShortUrlVisited; use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; +use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Model\Visitor; use Shlinkio\Shlink\Core\Model\VisitsParams; use Shlinkio\Shlink\Core\Paginator\Adapter\VisitsPaginatorAdapter; +use Shlinkio\Shlink\Core\Repository\ShortUrlRepositoryInterface; use Shlinkio\Shlink\Core\Repository\VisitRepository; class VisitsTracker implements VisitsTrackerInterface @@ -30,13 +32,8 @@ class VisitsTracker implements VisitsTrackerInterface /** * Tracks a new visit to provided short code from provided visitor */ - public function track(string $shortCode, Visitor $visitor): void + public function track(ShortUrl $shortUrl, Visitor $visitor): void { - /** @var ShortUrl $shortUrl */ - $shortUrl = $this->em->getRepository(ShortUrl::class)->findOneBy([ - 'shortCode' => $shortCode, - ]); - $visit = new Visit($shortUrl, $visitor); $this->em->persist($visit); @@ -51,17 +48,17 @@ class VisitsTracker implements VisitsTrackerInterface * @return Visit[]|Paginator * @throws ShortUrlNotFoundException */ - public function info(string $shortCode, VisitsParams $params): Paginator + public function info(ShortUrlIdentifier $identifier, VisitsParams $params): Paginator { - /** @var ORM\EntityRepository $repo */ + /** @var ShortUrlRepositoryInterface $repo */ $repo = $this->em->getRepository(ShortUrl::class); - if ($repo->count(['shortCode' => $shortCode]) < 1) { - throw ShortUrlNotFoundException::fromNotFoundShortCode($shortCode); + if (! $repo->shortCodeIsInUse($identifier->shortCode(), $identifier->domain())) { + throw ShortUrlNotFoundException::fromNotFound($identifier); } /** @var VisitRepository $repo */ $repo = $this->em->getRepository(Visit::class); - $paginator = new Paginator(new VisitsPaginatorAdapter($repo, $shortCode, $params)); + $paginator = new Paginator(new VisitsPaginatorAdapter($repo, $identifier, $params)); $paginator->setItemCountPerPage($params->getItemsPerPage()) ->setCurrentPageNumber($params->getPage()); diff --git a/module/Core/src/Service/VisitsTrackerInterface.php b/module/Core/src/Service/VisitsTrackerInterface.php index 5eddd96d..1ec4e110 100644 --- a/module/Core/src/Service/VisitsTrackerInterface.php +++ b/module/Core/src/Service/VisitsTrackerInterface.php @@ -5,8 +5,10 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Service; use Laminas\Paginator\Paginator; +use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Entity\Visit; use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; +use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Model\Visitor; use Shlinkio\Shlink\Core\Model\VisitsParams; @@ -15,7 +17,7 @@ interface VisitsTrackerInterface /** * Tracks a new visit to provided short code from provided visitor */ - public function track(string $shortCode, Visitor $visitor): void; + public function track(ShortUrl $shortUrl, Visitor $visitor): void; /** * Returns the visits on certain short code @@ -23,5 +25,5 @@ interface VisitsTrackerInterface * @return Visit[]|Paginator * @throws ShortUrlNotFoundException */ - public function info(string $shortCode, VisitsParams $params): Paginator; + public function info(ShortUrlIdentifier $identifier, VisitsParams $params): Paginator; } diff --git a/module/Core/src/Transformer/ShortUrlDataTransformer.php b/module/Core/src/Transformer/ShortUrlDataTransformer.php index 532fa122..a6fb4c14 100644 --- a/module/Core/src/Transformer/ShortUrlDataTransformer.php +++ b/module/Core/src/Transformer/ShortUrlDataTransformer.php @@ -24,16 +24,15 @@ class ShortUrlDataTransformer implements DataTransformerInterface */ public function transform($shortUrl): array // phpcs:ignore { - $longUrl = $shortUrl->getLongUrl(); - return [ 'shortCode' => $shortUrl->getShortCode(), 'shortUrl' => $shortUrl->toString($this->domainConfig), - 'longUrl' => $longUrl, + 'longUrl' => $shortUrl->getLongUrl(), 'dateCreated' => $shortUrl->getDateCreated()->toAtomString(), 'visitsCount' => $shortUrl->getVisitsCount(), 'tags' => invoke($shortUrl->getTags(), '__toString'), 'meta' => $this->buildMeta($shortUrl), + 'domain' => $shortUrl->getDomain(), ]; } diff --git a/module/Core/src/Validation/ShortUrlMetaInputFilter.php b/module/Core/src/Validation/ShortUrlMetaInputFilter.php index eca97a13..187ec66f 100644 --- a/module/Core/src/Validation/ShortUrlMetaInputFilter.php +++ b/module/Core/src/Validation/ShortUrlMetaInputFilter.php @@ -20,12 +20,10 @@ class ShortUrlMetaInputFilter extends InputFilter public const FIND_IF_EXISTS = 'findIfExists'; public const DOMAIN = 'domain'; - public function __construct(?array $data = null) + public function __construct(array $data) { $this->initialize(); - if ($data !== null) { - $this->setData($data); - } + $this->setData($data); } private function initialize(): void diff --git a/module/Core/src/Validation/ShortUrlsParamsInputFilter.php b/module/Core/src/Validation/ShortUrlsParamsInputFilter.php new file mode 100644 index 00000000..b3e6db2d --- /dev/null +++ b/module/Core/src/Validation/ShortUrlsParamsInputFilter.php @@ -0,0 +1,45 @@ +initialize(); + $this->setData($data); + } + + private function initialize(): void + { + $this->add($this->createDateInput(self::START_DATE, false)); + $this->add($this->createDateInput(self::END_DATE, false)); + + $this->add($this->createInput(self::SEARCH_TERM, false)); + + $page = $this->createInput(self::PAGE, false); + $page->getValidatorChain()->attach(new Validator\Digits()) + ->attach(new Validator\GreaterThan(['min' => 1, 'inclusive' => true])); + $this->add($page); + + $tags = $this->createArrayInput(self::TAGS, false); + $tags->getFilterChain()->attach(new Filter\StringToLower()) + ->attach(new Validation\SluggerFilter()); + $this->add($tags); + } +} diff --git a/module/Core/test-db/Repository/ShortUrlRepositoryTest.php b/module/Core/test-db/Repository/ShortUrlRepositoryTest.php index f742c6eb..4829fada 100644 --- a/module/Core/test-db/Repository/ShortUrlRepositoryTest.php +++ b/module/Core/test-db/Repository/ShortUrlRepositoryTest.php @@ -13,6 +13,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\ShortUrlsOrdering; use Shlinkio\Shlink\Core\Model\Visitor; use Shlinkio\Shlink\Core\Repository\ShortUrlRepository; use Shlinkio\Shlink\TestUtils\DbTest\DatabaseTestCase; @@ -36,7 +37,7 @@ class ShortUrlRepositoryTest extends DatabaseTestCase } /** @test */ - public function findOneByShortCodeReturnsProperData(): void + public function findOneWithDomainFallbackReturnsProperData(): void { $regularOne = new ShortUrl('foo', ShortUrlMeta::fromRawData(['customSlug' => 'foo'])); $this->getEntityManager()->persist($regularOne); @@ -53,20 +54,25 @@ class ShortUrlRepositoryTest extends DatabaseTestCase $this->getEntityManager()->flush(); - $this->assertSame($regularOne, $this->repo->findOneByShortCode($regularOne->getShortCode())); - $this->assertSame($regularOne, $this->repo->findOneByShortCode($withDomainDuplicatingRegular->getShortCode())); - $this->assertSame($withDomain, $this->repo->findOneByShortCode($withDomain->getShortCode(), 'example.com')); + $this->assertSame($regularOne, $this->repo->findOneWithDomainFallback($regularOne->getShortCode())); + $this->assertSame($regularOne, $this->repo->findOneWithDomainFallback( + $withDomainDuplicatingRegular->getShortCode(), + )); + $this->assertSame($withDomain, $this->repo->findOneWithDomainFallback( + $withDomain->getShortCode(), + 'example.com', + )); $this->assertSame( $withDomainDuplicatingRegular, - $this->repo->findOneByShortCode($withDomainDuplicatingRegular->getShortCode(), 'doma.in'), + $this->repo->findOneWithDomainFallback($withDomainDuplicatingRegular->getShortCode(), 'doma.in'), ); $this->assertSame( $regularOne, - $this->repo->findOneByShortCode($withDomainDuplicatingRegular->getShortCode(), 'other-domain.com'), + $this->repo->findOneWithDomainFallback($withDomainDuplicatingRegular->getShortCode(), 'other-domain.com'), ); - $this->assertNull($this->repo->findOneByShortCode('invalid')); - $this->assertNull($this->repo->findOneByShortCode($withDomain->getShortCode())); - $this->assertNull($this->repo->findOneByShortCode($withDomain->getShortCode(), 'other-domain.com')); + $this->assertNull($this->repo->findOneWithDomainFallback('invalid')); + $this->assertNull($this->repo->findOneWithDomainFallback($withDomain->getShortCode())); + $this->assertNull($this->repo->findOneWithDomainFallback($withDomain->getShortCode(), 'other-domain.com')); } /** @test */ @@ -122,7 +128,9 @@ class ShortUrlRepositoryTest extends DatabaseTestCase $this->assertCount(1, $this->repo->findList(2, 2)); - $result = $this->repo->findList(null, null, null, [], ['visits' => 'DESC']); + $result = $this->repo->findList(null, null, null, [], ShortUrlsOrdering::fromRawData([ + 'orderBy' => ['visits' => 'DESC'], + ])); $this->assertCount(3, $result); $this->assertSame($bar, $result[0]); @@ -148,7 +156,9 @@ class ShortUrlRepositoryTest extends DatabaseTestCase $this->getEntityManager()->flush(); - $result = $this->repo->findList(null, null, null, [], ['longUrl' => 'ASC']); + $result = $this->repo->findList(null, null, null, [], ShortUrlsOrdering::fromRawData([ + 'orderBy' => ['longUrl' => 'ASC'], + ])); $this->assertCount(count($urls), $result); $this->assertEquals('a', $result[0]->getLongUrl()); @@ -178,4 +188,26 @@ class ShortUrlRepositoryTest extends DatabaseTestCase $this->assertFalse($this->repo->shortCodeIsInUse('another-slug', 'example.com')); $this->assertTrue($this->repo->shortCodeIsInUse('another-slug', 'doma.in')); } + + /** @test */ + public function findOneLooksForShortUrlInProperSetOfTables(): void + { + $shortUrlWithoutDomain = new ShortUrl('foo', ShortUrlMeta::fromRawData(['customSlug' => 'my-cool-slug'])); + $this->getEntityManager()->persist($shortUrlWithoutDomain); + + $shortUrlWithDomain = new ShortUrl( + 'foo', + ShortUrlMeta::fromRawData(['domain' => 'doma.in', 'customSlug' => 'another-slug']), + ); + $this->getEntityManager()->persist($shortUrlWithDomain); + + $this->getEntityManager()->flush(); + + $this->assertNotNull($this->repo->findOne('my-cool-slug')); + $this->assertNull($this->repo->findOne('my-cool-slug', 'doma.in')); + $this->assertNull($this->repo->findOne('slug-not-in-use')); + $this->assertNull($this->repo->findOne('another-slug')); + $this->assertNull($this->repo->findOne('another-slug', 'example.com')); + $this->assertNotNull($this->repo->findOne('another-slug', 'doma.in')); + } } diff --git a/module/Core/test-db/Repository/VisitRepositoryTest.php b/module/Core/test-db/Repository/VisitRepositoryTest.php index 80207d5a..bd1a0f2d 100644 --- a/module/Core/test-db/Repository/VisitRepositoryTest.php +++ b/module/Core/test-db/Repository/VisitRepositoryTest.php @@ -6,9 +6,11 @@ namespace ShlinkioTest\Shlink\Core\Repository; use Cake\Chronos\Chronos; use Shlinkio\Shlink\Common\Util\DateRange; +use Shlinkio\Shlink\Core\Entity\Domain; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Entity\Visit; use Shlinkio\Shlink\Core\Entity\VisitLocation; +use Shlinkio\Shlink\Core\Model\ShortUrlMeta; use Shlinkio\Shlink\Core\Model\Visitor; use Shlinkio\Shlink\Core\Repository\VisitRepository; use Shlinkio\Shlink\IpGeolocation\Model\Location; @@ -24,6 +26,7 @@ class VisitRepositoryTest extends DatabaseTestCase VisitLocation::class, Visit::class, ShortUrl::class, + Domain::class, ]; private VisitRepository $repo; @@ -72,48 +75,73 @@ class VisitRepositoryTest extends DatabaseTestCase /** @test */ public function findVisitsByShortCodeReturnsProperData(): void { - $shortUrl = new ShortUrl(''); - $this->getEntityManager()->persist($shortUrl); - - for ($i = 0; $i < 6; $i++) { - $visit = new Visit($shortUrl, Visitor::emptyInstance(), Chronos::parse(sprintf('2016-01-0%s', $i + 1))); - $this->getEntityManager()->persist($visit); - } - $this->getEntityManager()->flush(); + [$shortCode, $domain] = $this->createShortUrlsAndVisits(); $this->assertCount(0, $this->repo->findVisitsByShortCode('invalid')); - $this->assertCount(6, $this->repo->findVisitsByShortCode($shortUrl->getShortCode())); - $this->assertCount(2, $this->repo->findVisitsByShortCode($shortUrl->getShortCode(), new DateRange( + $this->assertCount(6, $this->repo->findVisitsByShortCode($shortCode)); + $this->assertCount(3, $this->repo->findVisitsByShortCode($shortCode, $domain)); + $this->assertCount(2, $this->repo->findVisitsByShortCode($shortCode, null, new DateRange( Chronos::parse('2016-01-02'), Chronos::parse('2016-01-03'), ))); - $this->assertCount(4, $this->repo->findVisitsByShortCode($shortUrl->getShortCode(), new DateRange( + $this->assertCount(4, $this->repo->findVisitsByShortCode($shortCode, null, new DateRange( Chronos::parse('2016-01-03'), ))); - $this->assertCount(3, $this->repo->findVisitsByShortCode($shortUrl->getShortCode(), null, 3, 2)); - $this->assertCount(2, $this->repo->findVisitsByShortCode($shortUrl->getShortCode(), null, 5, 4)); + $this->assertCount(1, $this->repo->findVisitsByShortCode($shortCode, $domain, new DateRange( + Chronos::parse('2016-01-03'), + ))); + $this->assertCount(3, $this->repo->findVisitsByShortCode($shortCode, null, null, 3, 2)); + $this->assertCount(2, $this->repo->findVisitsByShortCode($shortCode, null, null, 5, 4)); + $this->assertCount(1, $this->repo->findVisitsByShortCode($shortCode, $domain, null, 3, 2)); } /** @test */ public function countVisitsByShortCodeReturnsProperData(): void + { + [$shortCode, $domain] = $this->createShortUrlsAndVisits(); + + $this->assertEquals(0, $this->repo->countVisitsByShortCode('invalid')); + $this->assertEquals(6, $this->repo->countVisitsByShortCode($shortCode)); + $this->assertEquals(3, $this->repo->countVisitsByShortCode($shortCode, $domain)); + $this->assertEquals(2, $this->repo->countVisitsByShortCode($shortCode, null, new DateRange( + Chronos::parse('2016-01-02'), + Chronos::parse('2016-01-03'), + ))); + $this->assertEquals(4, $this->repo->countVisitsByShortCode($shortCode, null, new DateRange( + Chronos::parse('2016-01-03'), + ))); + $this->assertEquals(1, $this->repo->countVisitsByShortCode($shortCode, $domain, new DateRange( + Chronos::parse('2016-01-03'), + ))); + } + + private function createShortUrlsAndVisits(): array { $shortUrl = new ShortUrl(''); + $domain = 'example.com'; + $shortCode = $shortUrl->getShortCode(); + $shortUrlWithDomain = new ShortUrl('', ShortUrlMeta::fromRawData([ + 'customSlug' => $shortCode, + 'domain' => $domain, + ])); + $this->getEntityManager()->persist($shortUrl); + $this->getEntityManager()->persist($shortUrlWithDomain); for ($i = 0; $i < 6; $i++) { $visit = new Visit($shortUrl, Visitor::emptyInstance(), Chronos::parse(sprintf('2016-01-0%s', $i + 1))); $this->getEntityManager()->persist($visit); } + for ($i = 0; $i < 3; $i++) { + $visit = new Visit( + $shortUrlWithDomain, + Visitor::emptyInstance(), + Chronos::parse(sprintf('2016-01-0%s', $i + 1)), + ); + $this->getEntityManager()->persist($visit); + } $this->getEntityManager()->flush(); - $this->assertEquals(0, $this->repo->countVisitsByShortCode('invalid')); - $this->assertEquals(6, $this->repo->countVisitsByShortCode($shortUrl->getShortCode())); - $this->assertEquals(2, $this->repo->countVisitsByShortCode($shortUrl->getShortCode(), new DateRange( - Chronos::parse('2016-01-02'), - Chronos::parse('2016-01-03'), - ))); - $this->assertEquals(4, $this->repo->countVisitsByShortCode($shortUrl->getShortCode(), new DateRange( - Chronos::parse('2016-01-03'), - ))); + return [$shortCode, $domain]; } } diff --git a/module/Core/test/Action/PixelActionTest.php b/module/Core/test/Action/PixelActionTest.php index ed9d0774..f4aed872 100644 --- a/module/Core/test/Action/PixelActionTest.php +++ b/module/Core/test/Action/PixelActionTest.php @@ -12,6 +12,7 @@ use Psr\Http\Server\RequestHandlerInterface; use Shlinkio\Shlink\Common\Response\PixelResponse; use Shlinkio\Shlink\Core\Action\PixelAction; use Shlinkio\Shlink\Core\Entity\ShortUrl; +use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Options\AppOptions; use Shlinkio\Shlink\Core\Service\ShortUrl\ShortUrlResolverInterface; use Shlinkio\Shlink\Core\Service\VisitsTracker; @@ -38,7 +39,7 @@ class PixelActionTest extends TestCase public function imageIsReturned(): void { $shortCode = 'abc123'; - $this->urlResolver->shortCodeToEnabledShortUrl($shortCode, '')->willReturn( + $this->urlResolver->resolveEnabledShortUrl(new ShortUrlIdentifier($shortCode, ''))->willReturn( new ShortUrl('http://domain.com/foo/bar'), )->shouldBeCalledOnce(); $this->visitTracker->track(Argument::cetera())->shouldBeCalledOnce(); diff --git a/module/Core/test/Action/QrCodeActionTest.php b/module/Core/test/Action/QrCodeActionTest.php index 63df94af..2a4d1a19 100644 --- a/module/Core/test/Action/QrCodeActionTest.php +++ b/module/Core/test/Action/QrCodeActionTest.php @@ -15,6 +15,7 @@ use Shlinkio\Shlink\Common\Response\QrCodeResponse; use Shlinkio\Shlink\Core\Action\QrCodeAction; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; +use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Service\ShortUrl\ShortUrlResolverInterface; class QrCodeActionTest extends TestCase @@ -36,8 +37,9 @@ class QrCodeActionTest extends TestCase public function aNotFoundShortCodeWillDelegateIntoNextMiddleware(): void { $shortCode = 'abc123'; - $this->urlResolver->shortCodeToEnabledShortUrl($shortCode, '')->willThrow(ShortUrlNotFoundException::class) - ->shouldBeCalledOnce(); + $this->urlResolver->resolveEnabledShortUrl(new ShortUrlIdentifier($shortCode, '')) + ->willThrow(ShortUrlNotFoundException::class) + ->shouldBeCalledOnce(); $delegate = $this->prophesize(RequestHandlerInterface::class); $process = $delegate->handle(Argument::any())->willReturn(new Response()); @@ -50,8 +52,9 @@ class QrCodeActionTest extends TestCase public function anInvalidShortCodeWillReturnNotFoundResponse(): void { $shortCode = 'abc123'; - $this->urlResolver->shortCodeToEnabledShortUrl($shortCode, '')->willThrow(ShortUrlNotFoundException::class) - ->shouldBeCalledOnce(); + $this->urlResolver->resolveEnabledShortUrl(new ShortUrlIdentifier($shortCode, '')) + ->willThrow(ShortUrlNotFoundException::class) + ->shouldBeCalledOnce(); $delegate = $this->prophesize(RequestHandlerInterface::class); $process = $delegate->handle(Argument::any())->willReturn(new Response()); @@ -64,8 +67,9 @@ class QrCodeActionTest extends TestCase public function aCorrectRequestReturnsTheQrCodeResponse(): void { $shortCode = 'abc123'; - $this->urlResolver->shortCodeToEnabledShortUrl($shortCode, '')->willReturn(new ShortUrl('')) - ->shouldBeCalledOnce(); + $this->urlResolver->resolveEnabledShortUrl(new ShortUrlIdentifier($shortCode, '')) + ->willReturn(new ShortUrl('')) + ->shouldBeCalledOnce(); $delegate = $this->prophesize(RequestHandlerInterface::class); $resp = $this->action->process( diff --git a/module/Core/test/Action/RedirectActionTest.php b/module/Core/test/Action/RedirectActionTest.php index 25e67f4b..8d28f21c 100644 --- a/module/Core/test/Action/RedirectActionTest.php +++ b/module/Core/test/Action/RedirectActionTest.php @@ -13,6 +13,7 @@ use Psr\Http\Server\RequestHandlerInterface; use Shlinkio\Shlink\Core\Action\RedirectAction; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; +use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Options; use Shlinkio\Shlink\Core\Service\ShortUrl\ShortUrlResolverInterface; use Shlinkio\Shlink\Core\Service\VisitsTrackerInterface; @@ -45,7 +46,8 @@ class RedirectActionTest extends TestCase { $shortCode = 'abc123'; $shortUrl = new ShortUrl('http://domain.com/foo/bar?some=thing'); - $shortCodeToUrl = $this->urlResolver->shortCodeToEnabledShortUrl($shortCode, '')->willReturn($shortUrl); + $shortCodeToUrl = $this->urlResolver->resolveEnabledShortUrl(new ShortUrlIdentifier($shortCode, '')) + ->willReturn($shortUrl); $track = $this->visitTracker->track(Argument::cetera())->will(function (): void { }); @@ -74,8 +76,9 @@ class RedirectActionTest extends TestCase public function nextMiddlewareIsInvokedIfLongUrlIsNotFound(): void { $shortCode = 'abc123'; - $this->urlResolver->shortCodeToEnabledShortUrl($shortCode, '')->willThrow(ShortUrlNotFoundException::class) - ->shouldBeCalledOnce(); + $this->urlResolver->resolveEnabledShortUrl(new ShortUrlIdentifier($shortCode, '')) + ->willThrow(ShortUrlNotFoundException::class) + ->shouldBeCalledOnce(); $this->visitTracker->track(Argument::cetera())->shouldNotBeCalled(); $handler = $this->prophesize(RequestHandlerInterface::class); diff --git a/module/Core/test/Exception/ShortUrlNotFoundExceptionTest.php b/module/Core/test/Exception/ShortUrlNotFoundExceptionTest.php index be02a66c..d0d77fb8 100644 --- a/module/Core/test/Exception/ShortUrlNotFoundExceptionTest.php +++ b/module/Core/test/Exception/ShortUrlNotFoundExceptionTest.php @@ -6,6 +6,7 @@ namespace ShlinkioTest\Shlink\Core\Exception; use PHPUnit\Framework\TestCase; use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; +use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; class ShortUrlNotFoundExceptionTest extends TestCase { @@ -23,7 +24,7 @@ class ShortUrlNotFoundExceptionTest extends TestCase $expectedAdditional['domain'] = $domain; } - $e = ShortUrlNotFoundException::fromNotFoundShortCode($shortCode, $domain); + $e = ShortUrlNotFoundException::fromNotFound(new ShortUrlIdentifier($shortCode, $domain)); $this->assertEquals($expectedMessage, $e->getMessage()); $this->assertEquals($expectedMessage, $e->getDetail()); diff --git a/module/Core/test/Paginator/Adapter/ShortUrlRepositoryAdapterTest.php b/module/Core/test/Paginator/Adapter/ShortUrlRepositoryAdapterTest.php index dd2fa24a..eb094c25 100644 --- a/module/Core/test/Paginator/Adapter/ShortUrlRepositoryAdapterTest.php +++ b/module/Core/test/Paginator/Adapter/ShortUrlRepositoryAdapterTest.php @@ -7,7 +7,7 @@ namespace ShlinkioTest\Shlink\Core\Paginator\Adapter; use Cake\Chronos\Chronos; use PHPUnit\Framework\TestCase; use Prophecy\Prophecy\ObjectProphecy; -use Shlinkio\Shlink\Common\Util\DateRange; +use Shlinkio\Shlink\Core\Model\ShortUrlsParams; use Shlinkio\Shlink\Core\Paginator\Adapter\ShortUrlRepositoryAdapter; use Shlinkio\Shlink\Core\Repository\ShortUrlRepositoryInterface; @@ -21,17 +21,26 @@ class ShortUrlRepositoryAdapterTest extends TestCase } /** - * @param string|array|null $orderBy * @test * @dataProvider provideFilteringArgs */ public function getItemsFallsBackToFindList( ?string $searchTerm = null, array $tags = [], - ?DateRange $dateRange = null, - $orderBy = null + ?string $startDate = null, + ?string $endDate = null, + ?string $orderBy = null ): void { - $adapter = new ShortUrlRepositoryAdapter($this->repo->reveal(), $searchTerm, $tags, $orderBy, $dateRange); + $params = ShortUrlsParams::fromRawData([ + 'searchTerm' => $searchTerm, + 'tags' => $tags, + 'startDate' => $startDate, + 'endDate' => $endDate, + 'orderBy' => $orderBy, + ]); + $adapter = new ShortUrlRepositoryAdapter($this->repo->reveal(), $params); + $orderBy = $params->orderBy(); + $dateRange = $params->dateRange(); $this->repo->findList(10, 5, $searchTerm, $tags, $orderBy, $dateRange)->shouldBeCalledOnce(); $adapter->getItems(5, 10); @@ -44,9 +53,17 @@ class ShortUrlRepositoryAdapterTest extends TestCase public function countFallsBackToCountList( ?string $searchTerm = null, array $tags = [], - ?DateRange $dateRange = null + ?string $startDate = null, + ?string $endDate = null ): void { - $adapter = new ShortUrlRepositoryAdapter($this->repo->reveal(), $searchTerm, $tags, null, $dateRange); + $params = ShortUrlsParams::fromRawData([ + 'searchTerm' => $searchTerm, + 'tags' => $tags, + 'startDate' => $startDate, + 'endDate' => $endDate, + ]); + $adapter = new ShortUrlRepositoryAdapter($this->repo->reveal(), $params); + $dateRange = $params->dateRange(); $this->repo->countList($searchTerm, $tags, $dateRange)->shouldBeCalledOnce(); $adapter->count(); @@ -58,12 +75,12 @@ class ShortUrlRepositoryAdapterTest extends TestCase yield ['search']; yield ['search', []]; yield ['search', ['foo', 'bar']]; - yield ['search', ['foo', 'bar'], null, 'order']; - yield ['search', ['foo', 'bar'], new DateRange(), 'order']; - yield ['search', ['foo', 'bar'], new DateRange(Chronos::now()), 'order']; - yield ['search', ['foo', 'bar'], new DateRange(null, Chronos::now()), 'order']; - yield ['search', ['foo', 'bar'], new DateRange(Chronos::now(), Chronos::now()), 'order']; - yield ['search', ['foo', 'bar'], new DateRange(Chronos::now())]; - yield [null, ['foo', 'bar'], new DateRange(Chronos::now(), Chronos::now())]; + yield ['search', ['foo', 'bar'], null, null, 'order']; + yield ['search', ['foo', 'bar'], Chronos::now()->toAtomString(), null, 'order']; + yield ['search', ['foo', 'bar'], null, Chronos::now()->toAtomString(), 'order']; + yield ['search', ['foo', 'bar'], Chronos::now()->toAtomString(), Chronos::now()->toAtomString(), 'order']; + yield [null, ['foo', 'bar'], Chronos::now()->toAtomString(), null, 'order']; + yield [null, ['foo', 'bar'], Chronos::now()->toAtomString()]; + yield [null, ['foo', 'bar'], Chronos::now()->toAtomString(), Chronos::now()->toAtomString()]; } } diff --git a/module/Core/test/Service/ShortUrl/DeleteShortUrlServiceTest.php b/module/Core/test/Service/ShortUrl/DeleteShortUrlServiceTest.php index e7aefd9d..0911cb7b 100644 --- a/module/Core/test/Service/ShortUrl/DeleteShortUrlServiceTest.php +++ b/module/Core/test/Service/ShortUrl/DeleteShortUrlServiceTest.php @@ -12,10 +12,11 @@ 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\ShortUrlIdentifier; 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; +use Shlinkio\Shlink\Core\Service\ShortUrl\ShortUrlResolverInterface; use function Functional\map; use function range; @@ -24,20 +25,20 @@ use function sprintf; class DeleteShortUrlServiceTest extends TestCase { private ObjectProphecy $em; + private ObjectProphecy $urlResolver; private string $shortCode; public function setUp(): void { - $shortUrl = (new ShortUrl(''))->setVisits( - new ArrayCollection(map(range(0, 10), fn () => new Visit(new ShortUrl(''), Visitor::emptyInstance()))), - ); + $shortUrl = (new ShortUrl(''))->setVisits(new ArrayCollection( + map(range(0, 10), fn () => new Visit(new ShortUrl(''), Visitor::emptyInstance())), + )); $this->shortCode = $shortUrl->getShortCode(); $this->em = $this->prophesize(EntityManagerInterface::class); - $repo = $this->prophesize(ShortUrlRepositoryInterface::class); - $repo->findOneBy(Argument::type('array'))->willReturn($shortUrl); - $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal()); + $this->urlResolver = $this->prophesize(ShortUrlResolverInterface::class); + $this->urlResolver->resolveShortUrl(Argument::cetera())->willReturn($shortUrl); } /** @test */ @@ -51,7 +52,7 @@ class DeleteShortUrlServiceTest extends TestCase $this->shortCode, )); - $service->deleteByShortCode($this->shortCode); + $service->deleteByShortCode(new ShortUrlIdentifier($this->shortCode)); } /** @test */ @@ -62,7 +63,7 @@ class DeleteShortUrlServiceTest extends TestCase $remove = $this->em->remove(Argument::type(ShortUrl::class))->willReturn(null); $flush = $this->em->flush()->willReturn(null); - $service->deleteByShortCode($this->shortCode, true); + $service->deleteByShortCode(new ShortUrlIdentifier($this->shortCode), true); $remove->shouldHaveBeenCalledOnce(); $flush->shouldHaveBeenCalledOnce(); @@ -76,7 +77,7 @@ class DeleteShortUrlServiceTest extends TestCase $remove = $this->em->remove(Argument::type(ShortUrl::class))->willReturn(null); $flush = $this->em->flush()->willReturn(null); - $service->deleteByShortCode($this->shortCode); + $service->deleteByShortCode(new ShortUrlIdentifier($this->shortCode)); $remove->shouldHaveBeenCalledOnce(); $flush->shouldHaveBeenCalledOnce(); @@ -90,7 +91,7 @@ class DeleteShortUrlServiceTest extends TestCase $remove = $this->em->remove(Argument::type(ShortUrl::class))->willReturn(null); $flush = $this->em->flush()->willReturn(null); - $service->deleteByShortCode($this->shortCode); + $service->deleteByShortCode(new ShortUrlIdentifier($this->shortCode)); $remove->shouldHaveBeenCalledOnce(); $flush->shouldHaveBeenCalledOnce(); @@ -101,6 +102,6 @@ class DeleteShortUrlServiceTest extends TestCase return new DeleteShortUrlService($this->em->reveal(), new DeleteShortUrlsOptions([ 'visitsThreshold' => $visitsThreshold, 'checkVisitsThreshold' => $checkVisitsThreshold, - ])); + ]), $this->urlResolver->reveal()); } } diff --git a/module/Core/test/Service/ShortUrl/ShortUrlResolverTest.php b/module/Core/test/Service/ShortUrl/ShortUrlResolverTest.php index 58f79606..5b3e3e19 100644 --- a/module/Core/test/Service/ShortUrl/ShortUrlResolverTest.php +++ b/module/Core/test/Service/ShortUrl/ShortUrlResolverTest.php @@ -12,6 +12,7 @@ use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Entity\Visit; use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; +use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Model\ShortUrlMeta; use Shlinkio\Shlink\Core\Model\Visitor; use Shlinkio\Shlink\Core\Repository\ShortUrlRepositoryInterface; @@ -38,13 +39,13 @@ class ShortUrlResolverTest extends TestCase $shortCode = $shortUrl->getShortCode(); $repo = $this->prophesize(ShortUrlRepositoryInterface::class); - $findOneByShortCode = $repo->findOneByShortCode($shortCode, null)->willReturn($shortUrl); + $findOne = $repo->findOne($shortCode, null)->willReturn($shortUrl); $getRepo = $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal()); - $result = $this->urlResolver->shortCodeToShortUrl($shortCode); + $result = $this->urlResolver->resolveShortUrl(new ShortUrlIdentifier($shortCode)); $this->assertSame($shortUrl, $result); - $findOneByShortCode->shouldHaveBeenCalledOnce(); + $findOne->shouldHaveBeenCalledOnce(); $getRepo->shouldHaveBeenCalledOnce(); } @@ -54,14 +55,14 @@ class ShortUrlResolverTest extends TestCase $shortCode = 'abc123'; $repo = $this->prophesize(ShortUrlRepositoryInterface::class); - $findOneByShortCode = $repo->findOneByShortCode($shortCode, null)->willReturn(null); + $findOne = $repo->findOne($shortCode, null)->willReturn(null); $getRepo = $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal()); $this->expectException(ShortUrlNotFoundException::class); - $findOneByShortCode->shouldBeCalledOnce(); + $findOne->shouldBeCalledOnce(); $getRepo->shouldBeCalledOnce(); - $this->urlResolver->shortCodeToShortUrl($shortCode); + $this->urlResolver->resolveShortUrl(new ShortUrlIdentifier($shortCode)); } /** @test */ @@ -71,10 +72,10 @@ class ShortUrlResolverTest extends TestCase $shortCode = $shortUrl->getShortCode(); $repo = $this->prophesize(ShortUrlRepositoryInterface::class); - $findOneByShortCode = $repo->findOneByShortCode($shortCode, null)->willReturn($shortUrl); + $findOneByShortCode = $repo->findOneWithDomainFallback($shortCode, null)->willReturn($shortUrl); $getRepo = $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal()); - $result = $this->urlResolver->shortCodeToEnabledShortUrl($shortCode); + $result = $this->urlResolver->resolveEnabledShortUrl(new ShortUrlIdentifier($shortCode)); $this->assertSame($shortUrl, $result); $findOneByShortCode->shouldHaveBeenCalledOnce(); @@ -90,14 +91,14 @@ class ShortUrlResolverTest extends TestCase $shortCode = $shortUrl->getShortCode(); $repo = $this->prophesize(ShortUrlRepositoryInterface::class); - $findOneByShortCode = $repo->findOneByShortCode($shortCode, null)->willReturn($shortUrl); + $findOneByShortCode = $repo->findOneWithDomainFallback($shortCode, null)->willReturn($shortUrl); $getRepo = $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal()); $this->expectException(ShortUrlNotFoundException::class); $findOneByShortCode->shouldBeCalledOnce(); $getRepo->shouldBeCalledOnce(); - $this->urlResolver->shortCodeToEnabledShortUrl($shortCode); + $this->urlResolver->resolveEnabledShortUrl(new ShortUrlIdentifier($shortCode)); } public function provideDisabledShortUrls(): iterable diff --git a/module/Core/test/Service/ShortUrlServiceTest.php b/module/Core/test/Service/ShortUrlServiceTest.php index 719c69d5..842eac60 100644 --- a/module/Core/test/Service/ShortUrlServiceTest.php +++ b/module/Core/test/Service/ShortUrlServiceTest.php @@ -12,9 +12,11 @@ use Prophecy\Argument; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Entity\Tag; -use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; +use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Model\ShortUrlMeta; +use Shlinkio\Shlink\Core\Model\ShortUrlsParams; use Shlinkio\Shlink\Core\Repository\ShortUrlRepository; +use Shlinkio\Shlink\Core\Service\ShortUrl\ShortUrlResolverInterface; use Shlinkio\Shlink\Core\Service\ShortUrlService; use function count; @@ -23,13 +25,17 @@ class ShortUrlServiceTest extends TestCase { private ShortUrlService $service; private ObjectProphecy $em; + private ObjectProphecy $urlResolver; public function setUp(): void { $this->em = $this->prophesize(EntityManagerInterface::class); $this->em->persist(Argument::any())->willReturn(null); $this->em->flush()->willReturn(null); - $this->service = new ShortUrlService($this->em->reveal()); + + $this->urlResolver = $this->prophesize(ShortUrlResolverInterface::class); + + $this->service = new ShortUrlService($this->em->reveal(), $this->urlResolver->reveal()); } /** @test */ @@ -47,40 +53,25 @@ class ShortUrlServiceTest extends TestCase $repo->countList(Argument::cetera())->willReturn(count($list))->shouldBeCalledOnce(); $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal()); - $list = $this->service->listShortUrls(); + $list = $this->service->listShortUrls(ShortUrlsParams::emptyInstance()); $this->assertEquals(4, $list->getCurrentItemCount()); } - /** @test */ - public function exceptionIsThrownWhenSettingTagsOnInvalidShortcode(): void - { - $shortCode = 'abc123'; - $repo = $this->prophesize(ShortUrlRepository::class); - $repo->findOneBy(['shortCode' => $shortCode])->willReturn(null) - ->shouldBeCalledOnce(); - $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal()); - - $this->expectException(ShortUrlNotFoundException::class); - $this->service->setTagsByShortCode($shortCode); - } - /** @test */ public function providedTagsAreGetFromRepoAndSetToTheShortUrl(): void { $shortUrl = $this->prophesize(ShortUrl::class); $shortUrl->setTags(Argument::any())->shouldBeCalledOnce(); $shortCode = 'abc123'; - $repo = $this->prophesize(ShortUrlRepository::class); - $repo->findOneBy(['shortCode' => $shortCode])->willReturn($shortUrl->reveal()) - ->shouldBeCalledOnce(); - $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal()); + $this->urlResolver->resolveShortUrl(new ShortUrlIdentifier($shortCode))->willReturn($shortUrl->reveal()) + ->shouldBeCalledOnce(); $tagRepo = $this->prophesize(EntityRepository::class); $tagRepo->findOneBy(['name' => 'foo'])->willReturn(new Tag('foo'))->shouldBeCalledOnce(); $tagRepo->findOneBy(['name' => 'bar'])->willReturn(null)->shouldBeCalledOnce(); $this->em->getRepository(Tag::class)->willReturn($tagRepo->reveal()); - $this->service->setTagsByShortCode($shortCode, ['foo', 'bar']); + $this->service->setTagsByShortCode(new ShortUrlIdentifier($shortCode), ['foo', 'bar']); } /** @test */ @@ -88,23 +79,22 @@ class ShortUrlServiceTest extends TestCase { $shortUrl = new ShortUrl(''); - $repo = $this->prophesize(ShortUrlRepository::class); - $findShortUrl = $repo->findOneBy(['shortCode' => 'abc123'])->willReturn($shortUrl); - $getRepo = $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal()); + $findShortUrl = $this->urlResolver->resolveShortUrl(new ShortUrlIdentifier('abc123'))->willReturn($shortUrl); $flush = $this->em->flush()->willReturn(null); - $result = $this->service->updateMetadataByShortCode('abc123', ShortUrlMeta::fromRawData([ - 'validSince' => Chronos::parse('2017-01-01 00:00:00')->toAtomString(), - 'validUntil' => Chronos::parse('2017-01-05 00:00:00')->toAtomString(), - 'maxVisits' => 5, - ])); + $result = $this->service->updateMetadataByShortCode(new ShortUrlIdentifier('abc123'), ShortUrlMeta::fromRawData( + [ + 'validSince' => Chronos::parse('2017-01-01 00:00:00')->toAtomString(), + 'validUntil' => Chronos::parse('2017-01-05 00:00:00')->toAtomString(), + 'maxVisits' => 5, + ], + )); $this->assertSame($shortUrl, $result); $this->assertEquals(Chronos::parse('2017-01-01 00:00:00'), $shortUrl->getValidSince()); $this->assertEquals(Chronos::parse('2017-01-05 00:00:00'), $shortUrl->getValidUntil()); $this->assertEquals(5, $shortUrl->getMaxVisits()); $findShortUrl->shouldHaveBeenCalled(); - $getRepo->shouldHaveBeenCalled(); $flush->shouldHaveBeenCalled(); } } diff --git a/module/Core/test/Service/VisitsTrackerTest.php b/module/Core/test/Service/VisitsTrackerTest.php index 5cf78b16..9028d2c7 100644 --- a/module/Core/test/Service/VisitsTrackerTest.php +++ b/module/Core/test/Service/VisitsTrackerTest.php @@ -5,7 +5,6 @@ declare(strict_types=1); namespace ShlinkioTest\Shlink\Core\Service; use Doctrine\ORM\EntityManager; -use Doctrine\ORM\EntityRepository; use Laminas\Stdlib\ArrayUtils; use PHPUnit\Framework\Assert; use PHPUnit\Framework\TestCase; @@ -16,11 +15,17 @@ use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Entity\Visit; use Shlinkio\Shlink\Core\EventDispatcher\ShortUrlVisited; +use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; +use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Model\Visitor; use Shlinkio\Shlink\Core\Model\VisitsParams; +use Shlinkio\Shlink\Core\Repository\ShortUrlRepositoryInterface; use Shlinkio\Shlink\Core\Repository\VisitRepository; use Shlinkio\Shlink\Core\Service\VisitsTracker; +use function Functional\map; +use function range; + class VisitsTrackerTest extends TestCase { private VisitsTracker $visitsTracker; @@ -39,14 +44,11 @@ class VisitsTrackerTest extends TestCase public function trackPersistsVisit(): void { $shortCode = '123ABC'; - $repo = $this->prophesize(EntityRepository::class); - $repo->findOneBy(['shortCode' => $shortCode])->willReturn(new ShortUrl('')); - $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal())->shouldBeCalledOnce(); $this->em->persist(Argument::that(fn (Visit $visit) => $visit->setId('1')))->shouldBeCalledOnce(); $this->em->flush()->shouldBeCalledOnce(); - $this->visitsTracker->track($shortCode, Visitor::emptyInstance()); + $this->visitsTracker->track(new ShortUrl($shortCode), Visitor::emptyInstance()); $this->eventDispatcher->dispatch(Argument::type(ShortUrlVisited::class))->shouldHaveBeenCalled(); } @@ -55,10 +57,7 @@ class VisitsTrackerTest extends TestCase public function trackedIpAddressGetsObfuscated(): void { $shortCode = '123ABC'; - $repo = $this->prophesize(EntityRepository::class); - $repo->findOneBy(['shortCode' => $shortCode])->willReturn(new ShortUrl('')); - $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal())->shouldBeCalledOnce(); $this->em->persist(Argument::any())->will(function ($args) { /** @var Visit $visit */ $visit = $args[0]; @@ -68,7 +67,7 @@ class VisitsTrackerTest extends TestCase })->shouldBeCalledOnce(); $this->em->flush()->shouldBeCalledOnce(); - $this->visitsTracker->track($shortCode, new Visitor('', '', '4.3.2.1')); + $this->visitsTracker->track(new ShortUrl($shortCode), new Visitor('', '', '4.3.2.1')); $this->eventDispatcher->dispatch(Argument::type(ShortUrlVisited::class))->shouldHaveBeenCalled(); } @@ -77,22 +76,33 @@ class VisitsTrackerTest extends TestCase public function infoReturnsVisitsForCertainShortCode(): void { $shortCode = '123ABC'; - $repo = $this->prophesize(EntityRepository::class); - $count = $repo->count(['shortCode' => $shortCode])->willReturn(1); + $repo = $this->prophesize(ShortUrlRepositoryInterface::class); + $count = $repo->shortCodeIsInUse($shortCode, null)->willReturn(true); $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal())->shouldBeCalledOnce(); - $list = [ - new Visit(new ShortUrl(''), Visitor::emptyInstance()), - new Visit(new ShortUrl(''), Visitor::emptyInstance()), - ]; + $list = map(range(0, 1), fn () => new Visit(new ShortUrl(''), Visitor::emptyInstance())); $repo2 = $this->prophesize(VisitRepository::class); - $repo2->findVisitsByShortCode($shortCode, Argument::type(DateRange::class), 1, 0)->willReturn($list); - $repo2->countVisitsByShortCode($shortCode, Argument::type(DateRange::class))->willReturn(1); + $repo2->findVisitsByShortCode($shortCode, null, Argument::type(DateRange::class), 1, 0)->willReturn($list); + $repo2->countVisitsByShortCode($shortCode, null, Argument::type(DateRange::class))->willReturn(1); $this->em->getRepository(Visit::class)->willReturn($repo2->reveal())->shouldBeCalledOnce(); - $paginator = $this->visitsTracker->info($shortCode, new VisitsParams()); + $paginator = $this->visitsTracker->info(new ShortUrlIdentifier($shortCode), new VisitsParams()); $this->assertEquals($list, ArrayUtils::iteratorToArray($paginator->getCurrentItems())); $count->shouldHaveBeenCalledOnce(); } + + /** @test */ + public function throwsExceptionWhenRequestingVisitsForInvalidShortCode(): void + { + $shortCode = '123ABC'; + $repo = $this->prophesize(ShortUrlRepositoryInterface::class); + $count = $repo->shortCodeIsInUse($shortCode, null)->willReturn(false); + $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal())->shouldBeCalledOnce(); + + $this->expectException(ShortUrlNotFoundException::class); + $count->shouldBeCalledOnce(); + + $this->visitsTracker->info(new ShortUrlIdentifier($shortCode), new VisitsParams()); + } } diff --git a/module/Rest/config/dependencies.config.php b/module/Rest/config/dependencies.config.php index 6938b6ba..0058b100 100644 --- a/module/Rest/config/dependencies.config.php +++ b/module/Rest/config/dependencies.config.php @@ -37,6 +37,7 @@ return [ Middleware\BodyParserMiddleware::class => InvokableFactory::class, Middleware\CrossDomainMiddleware::class => InvokableFactory::class, Middleware\ShortUrl\CreateShortUrlContentNegotiationMiddleware::class => InvokableFactory::class, + Middleware\ShortUrl\DropDefaultDomainFromQueryMiddleware::class => ConfigAbstractFactory::class, ], ], @@ -72,6 +73,8 @@ return [ Action\Tag\DeleteTagsAction::class => [Service\Tag\TagService::class, LoggerInterface::class], Action\Tag\CreateTagsAction::class => [Service\Tag\TagService::class, LoggerInterface::class], Action\Tag\UpdateTagAction::class => [Service\Tag\TagService::class, LoggerInterface::class], + + Middleware\ShortUrl\DropDefaultDomainFromQueryMiddleware::class => ['config.url_shortener.domain.hostname'], ], ]; diff --git a/module/Rest/config/entities-mappings/Shlinkio.Shlink.Rest.Entity.ApiKey.php b/module/Rest/config/entities-mappings/Shlinkio.Shlink.Rest.Entity.ApiKey.php index 5bc5aec9..a5084cee 100644 --- a/module/Rest/config/entities-mappings/Shlinkio.Shlink.Rest.Entity.ApiKey.php +++ b/module/Rest/config/entities-mappings/Shlinkio.Shlink.Rest.Entity.ApiKey.php @@ -6,29 +6,32 @@ namespace Shlinkio\Shlink\Rest; use Doctrine\DBAL\Types\Types; use Doctrine\ORM\Mapping\Builder\ClassMetadataBuilder; -use Doctrine\ORM\Mapping\ClassMetadata; // @codingStandardsIgnoreLine +use Doctrine\ORM\Mapping\ClassMetadata; use Shlinkio\Shlink\Common\Doctrine\Type\ChronosDateTimeType; -/** @var $metadata ClassMetadata */ // @codingStandardsIgnoreLine -$builder = new ClassMetadataBuilder($metadata); +use function Shlinkio\Shlink\Core\determineTableName; -$builder->setTable('api_keys'); +return static function (ClassMetadata $metadata, array $emConfig): void { + $builder = new ClassMetadataBuilder($metadata); -$builder->createField('id', Types::BIGINT) - ->makePrimaryKey() - ->generatedValue('IDENTITY') - ->option('unsigned', true) - ->build(); + $builder->setTable(determineTableName('api_keys', $emConfig)); -$builder->createField('key', Types::STRING) - ->columnName('`key`') - ->unique() - ->build(); + $builder->createField('id', Types::BIGINT) + ->makePrimaryKey() + ->generatedValue('IDENTITY') + ->option('unsigned', true) + ->build(); -$builder->createField('expirationDate', ChronosDateTimeType::CHRONOS_DATETIME) - ->columnName('expiration_date') - ->nullable() - ->build(); + $builder->createField('key', Types::STRING) + ->columnName('`key`') + ->unique() + ->build(); -$builder->createField('enabled', Types::BOOLEAN) - ->build(); + $builder->createField('expirationDate', ChronosDateTimeType::CHRONOS_DATETIME) + ->columnName('expiration_date') + ->nullable() + ->build(); + + $builder->createField('enabled', Types::BOOLEAN) + ->build(); +}; diff --git a/module/Rest/config/routes.config.php b/module/Rest/config/routes.config.php index d210f13b..301691aa 100644 --- a/module/Rest/config/routes.config.php +++ b/module/Rest/config/routes.config.php @@ -4,26 +4,25 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Rest; +$contentNegotiationMiddleware = [Middleware\ShortUrl\CreateShortUrlContentNegotiationMiddleware::class]; +$dropDomainMiddleware = [Middleware\ShortUrl\DropDefaultDomainFromQueryMiddleware::class]; + return [ 'routes' => [ Action\HealthAction::getRouteDef(), // Short codes - Action\ShortUrl\CreateShortUrlAction::getRouteDef([ - Middleware\ShortUrl\CreateShortUrlContentNegotiationMiddleware::class, - ]), - Action\ShortUrl\SingleStepCreateShortUrlAction::getRouteDef([ - Middleware\ShortUrl\CreateShortUrlContentNegotiationMiddleware::class, - ]), - Action\ShortUrl\EditShortUrlAction::getRouteDef(), - Action\ShortUrl\DeleteShortUrlAction::getRouteDef(), - Action\ShortUrl\ResolveShortUrlAction::getRouteDef(), + Action\ShortUrl\CreateShortUrlAction::getRouteDef($contentNegotiationMiddleware), + Action\ShortUrl\SingleStepCreateShortUrlAction::getRouteDef($contentNegotiationMiddleware), + Action\ShortUrl\EditShortUrlAction::getRouteDef($dropDomainMiddleware), + Action\ShortUrl\DeleteShortUrlAction::getRouteDef($dropDomainMiddleware), + Action\ShortUrl\ResolveShortUrlAction::getRouteDef($dropDomainMiddleware), Action\ShortUrl\ListShortUrlsAction::getRouteDef(), - Action\ShortUrl\EditShortUrlTagsAction::getRouteDef(), + Action\ShortUrl\EditShortUrlTagsAction::getRouteDef($dropDomainMiddleware), // Visits - Action\Visit\GetVisitsAction::getRouteDef(), + Action\Visit\GetVisitsAction::getRouteDef($dropDomainMiddleware), // Tags Action\Tag\ListTagsAction::getRouteDef(), diff --git a/module/Rest/src/Action/ShortUrl/DeleteShortUrlAction.php b/module/Rest/src/Action/ShortUrl/DeleteShortUrlAction.php index e0b269f3..d86c60e9 100644 --- a/module/Rest/src/Action/ShortUrl/DeleteShortUrlAction.php +++ b/module/Rest/src/Action/ShortUrl/DeleteShortUrlAction.php @@ -8,6 +8,7 @@ use Laminas\Diactoros\Response\EmptyResponse; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; use Psr\Log\LoggerInterface; +use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Service\ShortUrl\DeleteShortUrlServiceInterface; use Shlinkio\Shlink\Rest\Action\AbstractRestAction; @@ -26,8 +27,8 @@ class DeleteShortUrlAction extends AbstractRestAction public function handle(ServerRequestInterface $request): ResponseInterface { - $shortCode = $request->getAttribute('shortCode', ''); - $this->deleteShortUrlService->deleteByShortCode($shortCode); + $identifier = ShortUrlIdentifier::fromApiRequest($request); + $this->deleteShortUrlService->deleteByShortCode($identifier); return new EmptyResponse(); } } diff --git a/module/Rest/src/Action/ShortUrl/EditShortUrlAction.php b/module/Rest/src/Action/ShortUrl/EditShortUrlAction.php index a6bc5538..8b3e65ab 100644 --- a/module/Rest/src/Action/ShortUrl/EditShortUrlAction.php +++ b/module/Rest/src/Action/ShortUrl/EditShortUrlAction.php @@ -8,6 +8,7 @@ use Laminas\Diactoros\Response\EmptyResponse; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; use Psr\Log\LoggerInterface; +use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Model\ShortUrlMeta; use Shlinkio\Shlink\Core\Service\ShortUrlServiceInterface; use Shlinkio\Shlink\Rest\Action\AbstractRestAction; @@ -28,9 +29,9 @@ class EditShortUrlAction extends AbstractRestAction public function handle(ServerRequestInterface $request): ResponseInterface { $postData = (array) $request->getParsedBody(); - $shortCode = $request->getAttribute('shortCode', ''); + $identifier = ShortUrlIdentifier::fromApiRequest($request); - $this->shortUrlService->updateMetadataByShortCode($shortCode, ShortUrlMeta::fromRawData($postData)); + $this->shortUrlService->updateMetadataByShortCode($identifier, ShortUrlMeta::fromRawData($postData)); return new EmptyResponse(); } } diff --git a/module/Rest/src/Action/ShortUrl/EditShortUrlTagsAction.php b/module/Rest/src/Action/ShortUrl/EditShortUrlTagsAction.php index e30710ed..0a48d986 100644 --- a/module/Rest/src/Action/ShortUrl/EditShortUrlTagsAction.php +++ b/module/Rest/src/Action/ShortUrl/EditShortUrlTagsAction.php @@ -9,6 +9,7 @@ use Psr\Http\Message\ResponseInterface as Response; use Psr\Http\Message\ServerRequestInterface as Request; use Psr\Log\LoggerInterface; use Shlinkio\Shlink\Core\Exception\ValidationException; +use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Service\ShortUrlServiceInterface; use Shlinkio\Shlink\Rest\Action\AbstractRestAction; @@ -27,7 +28,6 @@ class EditShortUrlTagsAction extends AbstractRestAction public function handle(Request $request): Response { - $shortCode = $request->getAttribute('shortCode'); $bodyParams = $request->getParsedBody(); if (! isset($bodyParams['tags'])) { @@ -35,9 +35,10 @@ class EditShortUrlTagsAction extends AbstractRestAction 'tags' => 'List of tags has to be provided', ]); } - $tags = $bodyParams['tags']; + ['tags' => $tags] = $bodyParams; + $identifier = ShortUrlIdentifier::fromApiRequest($request); - $shortUrl = $this->shortUrlService->setTagsByShortCode($shortCode, $tags); + $shortUrl = $this->shortUrlService->setTagsByShortCode($identifier, $tags); return new JsonResponse(['tags' => $shortUrl->getTags()->toArray()]); } } diff --git a/module/Rest/src/Action/ShortUrl/ListShortUrlsAction.php b/module/Rest/src/Action/ShortUrl/ListShortUrlsAction.php index a50493db..5801eeec 100644 --- a/module/Rest/src/Action/ShortUrl/ListShortUrlsAction.php +++ b/module/Rest/src/Action/ShortUrl/ListShortUrlsAction.php @@ -4,14 +4,12 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Rest\Action\ShortUrl; -use Cake\Chronos\Chronos; -use InvalidArgumentException; use Laminas\Diactoros\Response\JsonResponse; use Psr\Http\Message\ResponseInterface as Response; use Psr\Http\Message\ServerRequestInterface as Request; use Psr\Log\LoggerInterface; use Shlinkio\Shlink\Common\Paginator\Util\PaginatorUtilsTrait; -use Shlinkio\Shlink\Common\Util\DateRange; +use Shlinkio\Shlink\Core\Model\ShortUrlsParams; use Shlinkio\Shlink\Core\Service\ShortUrlServiceInterface; use Shlinkio\Shlink\Core\Transformer\ShortUrlDataTransformer; use Shlinkio\Shlink\Rest\Action\AbstractRestAction; @@ -36,38 +34,11 @@ class ListShortUrlsAction extends AbstractRestAction $this->domainConfig = $domainConfig; } - /** - * @throws InvalidArgumentException - */ public function handle(Request $request): Response { - $params = $this->queryToListParams($request->getQueryParams()); - $shortUrls = $this->shortUrlService->listShortUrls(...$params); + $shortUrls = $this->shortUrlService->listShortUrls(ShortUrlsParams::fromRawData($request->getQueryParams())); return new JsonResponse(['shortUrls' => $this->serializePaginator($shortUrls, new ShortUrlDataTransformer( $this->domainConfig, ))]); } - - /** - * @param array $query - * @return array - */ - private function queryToListParams(array $query): array - { - return [ - (int) ($query['page'] ?? 1), - $query['searchTerm'] ?? null, - $query['tags'] ?? [], - $query['orderBy'] ?? null, - $this->determineDateRangeFromQuery($query), - ]; - } - - private function determineDateRangeFromQuery(array $query): DateRange - { - return new DateRange( - isset($query['startDate']) ? Chronos::parse($query['startDate']) : null, - isset($query['endDate']) ? Chronos::parse($query['endDate']) : null, - ); - } } diff --git a/module/Rest/src/Action/ShortUrl/ResolveShortUrlAction.php b/module/Rest/src/Action/ShortUrl/ResolveShortUrlAction.php index 03458a2c..41cd2b2d 100644 --- a/module/Rest/src/Action/ShortUrl/ResolveShortUrlAction.php +++ b/module/Rest/src/Action/ShortUrl/ResolveShortUrlAction.php @@ -8,6 +8,7 @@ use Laminas\Diactoros\Response\JsonResponse; use Psr\Http\Message\ResponseInterface as Response; use Psr\Http\Message\ServerRequestInterface as Request; use Psr\Log\LoggerInterface; +use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Service\ShortUrl\ShortUrlResolverInterface; use Shlinkio\Shlink\Core\Transformer\ShortUrlDataTransformer; use Shlinkio\Shlink\Rest\Action\AbstractRestAction; @@ -32,11 +33,9 @@ class ResolveShortUrlAction extends AbstractRestAction public function handle(Request $request): Response { - $shortCode = $request->getAttribute('shortCode'); - $domain = $request->getQueryParams()['domain'] ?? null; $transformer = new ShortUrlDataTransformer($this->domainConfig); + $url = $this->urlResolver->resolveShortUrl(ShortUrlIdentifier::fromApiRequest($request)); - $url = $this->urlResolver->shortCodeToShortUrl($shortCode, $domain); return new JsonResponse($transformer->transform($url)); } } diff --git a/module/Rest/src/Action/Visit/GetVisitsAction.php b/module/Rest/src/Action/Visit/GetVisitsAction.php index c1fa0095..bd6ae5a5 100644 --- a/module/Rest/src/Action/Visit/GetVisitsAction.php +++ b/module/Rest/src/Action/Visit/GetVisitsAction.php @@ -9,6 +9,7 @@ use Psr\Http\Message\ResponseInterface as Response; use Psr\Http\Message\ServerRequestInterface as Request; use Psr\Log\LoggerInterface; use Shlinkio\Shlink\Common\Paginator\Util\PaginatorUtilsTrait; +use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Model\VisitsParams; use Shlinkio\Shlink\Core\Service\VisitsTrackerInterface; use Shlinkio\Shlink\Rest\Action\AbstractRestAction; @@ -30,8 +31,8 @@ class GetVisitsAction extends AbstractRestAction public function handle(Request $request): Response { - $shortCode = $request->getAttribute('shortCode'); - $visits = $this->visitsTracker->info($shortCode, VisitsParams::fromRawData($request->getQueryParams())); + $identifier = ShortUrlIdentifier::fromApiRequest($request); + $visits = $this->visitsTracker->info($identifier, VisitsParams::fromRawData($request->getQueryParams())); return new JsonResponse([ 'visits' => $this->serializePaginator($visits), diff --git a/module/Rest/src/Middleware/ShortUrl/DropDefaultDomainFromQueryMiddleware.php b/module/Rest/src/Middleware/ShortUrl/DropDefaultDomainFromQueryMiddleware.php new file mode 100644 index 00000000..b894e40c --- /dev/null +++ b/module/Rest/src/Middleware/ShortUrl/DropDefaultDomainFromQueryMiddleware.php @@ -0,0 +1,31 @@ +defaultDomain = $defaultDomain; + } + + public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface + { + $query = $request->getQueryParams(); + if (isset($query['domain']) && $query['domain'] === $this->defaultDomain) { + unset($query['domain']); + $request = $request->withQueryParams($query); + } + + return $handler->handle($request); + } +} diff --git a/module/Rest/test-api/Action/DeleteShortUrlActionTest.php b/module/Rest/test-api/Action/DeleteShortUrlActionTest.php index 7bf01c51..ef32190b 100644 --- a/module/Rest/test-api/Action/DeleteShortUrlActionTest.php +++ b/module/Rest/test-api/Action/DeleteShortUrlActionTest.php @@ -5,15 +5,22 @@ declare(strict_types=1); namespace ShlinkioApiTest\Shlink\Rest\Action; use Shlinkio\Shlink\TestUtils\ApiTest\ApiTestCase; +use ShlinkioApiTest\Shlink\Rest\Utils\NotFoundUrlHelpersTrait; class DeleteShortUrlActionTest extends ApiTestCase { - /** @test */ - public function notFoundErrorIsReturnWhenDeletingInvalidUrl(): void - { - $expectedDetail = 'No URL found with short code "invalid"'; + use NotFoundUrlHelpersTrait; - $resp = $this->callApiWithKey(self::METHOD_DELETE, '/short-urls/invalid'); + /** + * @test + * @dataProvider provideInvalidUrls + */ + public function notFoundErrorIsReturnWhenDeletingInvalidUrl( + string $shortCode, + ?string $domain, + string $expectedDetail + ): void { + $resp = $this->callApiWithKey(self::METHOD_DELETE, $this->buildShortUrlPath($shortCode, $domain)); $payload = $this->getJsonResponsePayload($resp); $this->assertEquals(self::STATUS_NOT_FOUND, $resp->getStatusCode()); @@ -21,7 +28,8 @@ class DeleteShortUrlActionTest extends ApiTestCase $this->assertEquals('INVALID_SHORTCODE', $payload['type']); $this->assertEquals($expectedDetail, $payload['detail']); $this->assertEquals('Short URL not found', $payload['title']); - $this->assertEquals('invalid', $payload['shortCode']); + $this->assertEquals($shortCode, $payload['shortCode']); + $this->assertEquals($domain, $payload['domain'] ?? null); } /** @test */ @@ -42,4 +50,20 @@ class DeleteShortUrlActionTest extends ApiTestCase $this->assertEquals($expectedDetail, $payload['detail']); $this->assertEquals('Cannot delete short URL', $payload['title']); } + + /** @test */ + public function properShortUrlIsDeletedWhenDomainIsProvided(): void + { + $fetchWithDomainBefore = $this->callApiWithKey(self::METHOD_GET, '/short-urls/ghi789?domain=example.com'); + $fetchWithoutDomainBefore = $this->callApiWithKey(self::METHOD_GET, '/short-urls/ghi789'); + $deleteResp = $this->callApiWithKey(self::METHOD_DELETE, '/short-urls/ghi789?domain=example.com'); + $fetchWithDomainAfter = $this->callApiWithKey(self::METHOD_GET, '/short-urls/ghi789?domain=example.com'); + $fetchWithoutDomainAfter = $this->callApiWithKey(self::METHOD_GET, '/short-urls/ghi789'); + + $this->assertEquals(self::STATUS_OK, $fetchWithDomainBefore->getStatusCode()); + $this->assertEquals(self::STATUS_OK, $fetchWithoutDomainBefore->getStatusCode()); + $this->assertEquals(self::STATUS_NO_CONTENT, $deleteResp->getStatusCode()); + $this->assertEquals(self::STATUS_NOT_FOUND, $fetchWithDomainAfter->getStatusCode()); + $this->assertEquals(self::STATUS_OK, $fetchWithoutDomainAfter->getStatusCode()); + } } diff --git a/module/Rest/test-api/Action/EditShortUrlActionTest.php b/module/Rest/test-api/Action/EditShortUrlActionTest.php index 482accfe..171a40cc 100644 --- a/module/Rest/test-api/Action/EditShortUrlActionTest.php +++ b/module/Rest/test-api/Action/EditShortUrlActionTest.php @@ -7,14 +7,17 @@ namespace ShlinkioApiTest\Shlink\Rest\Action; use Cake\Chronos\Chronos; use DMS\PHPUnitExtensions\ArraySubset\ArraySubsetAsserts; use GuzzleHttp\RequestOptions; +use Laminas\Diactoros\Uri; use Shlinkio\Shlink\TestUtils\ApiTest\ApiTestCase; +use ShlinkioApiTest\Shlink\Rest\Utils\NotFoundUrlHelpersTrait; -use function Functional\first; +use function GuzzleHttp\Psr7\build_query; use function sprintf; class EditShortUrlActionTest extends ApiTestCase { use ArraySubsetAsserts; + use NotFoundUrlHelpersTrait; /** * @test @@ -61,20 +64,24 @@ class EditShortUrlActionTest extends ApiTestCase private function findShortUrlMetaByShortCode(string $shortCode): ?array { - // FIXME Call GET /short-urls/{shortCode} once issue https://github.com/shlinkio/shlink/issues/628 is fixed - $allShortUrls = $this->getJsonResponsePayload($this->callApiWithKey(self::METHOD_GET, '/short-urls')); - $list = $allShortUrls['shortUrls']['data'] ?? []; - $matchingShortUrl = first($list, fn (array $shortUrl) => $shortUrl['shortCode'] ?? '' === $shortCode); + $matchingShortUrl = $this->getJsonResponsePayload( + $this->callApiWithKey(self::METHOD_GET, '/short-urls/' . $shortCode), + ); return $matchingShortUrl['meta'] ?? null; } - /** @test */ - public function tryingToEditInvalidUrlReturnsNotFoundError(): void - { - $expectedDetail = 'No URL found with short code "invalid"'; - - $resp = $this->callApiWithKey(self::METHOD_PATCH, '/short-urls/invalid', [RequestOptions::JSON => []]); + /** + * @test + * @dataProvider provideInvalidUrls + */ + public function tryingToEditInvalidUrlReturnsNotFoundError( + string $shortCode, + ?string $domain, + string $expectedDetail + ): void { + $url = $this->buildShortUrlPath($shortCode, $domain); + $resp = $this->callApiWithKey(self::METHOD_PATCH, $url, [RequestOptions::JSON => []]); $payload = $this->getJsonResponsePayload($resp); $this->assertEquals(self::STATUS_NOT_FOUND, $resp->getStatusCode()); @@ -82,7 +89,8 @@ class EditShortUrlActionTest extends ApiTestCase $this->assertEquals('INVALID_SHORTCODE', $payload['type']); $this->assertEquals($expectedDetail, $payload['detail']); $this->assertEquals('Short URL not found', $payload['title']); - $this->assertEquals('invalid', $payload['shortCode']); + $this->assertEquals($shortCode, $payload['shortCode']); + $this->assertEquals($domain, $payload['domain'] ?? null); } /** @test */ @@ -101,4 +109,37 @@ class EditShortUrlActionTest extends ApiTestCase $this->assertEquals($expectedDetail, $payload['detail']); $this->assertEquals('Invalid data', $payload['title']); } + + /** + * @test + * @dataProvider provideDomains + */ + public function metadataIsEditedOnProperShortUrlBasedOnDomain(?string $domain, string $expectedUrl): void + { + $shortCode = 'ghi789'; + $url = new Uri(sprintf('/short-urls/%s', $shortCode)); + + if ($domain !== null) { + $url = $url->withQuery(build_query(['domain' => $domain])); + } + + $editResp = $this->callApiWithKey(self::METHOD_PATCH, (string) $url, [RequestOptions::JSON => [ + 'maxVisits' => 100, + ]]); + $editedShortUrl = $this->getJsonResponsePayload($this->callApiWithKey(self::METHOD_GET, (string) $url)); + + $this->assertEquals(self::STATUS_NO_CONTENT, $editResp->getStatusCode()); + $this->assertEquals($domain, $editedShortUrl['domain']); + $this->assertEquals($expectedUrl, $editedShortUrl['longUrl']); + $this->assertEquals(100, $editedShortUrl['meta']['maxVisits'] ?? null); + } + + public function provideDomains(): iterable + { + yield 'domain' => [ + 'example.com', + 'https://blog.alejandrocelaya.com/2019/04/27/considerations-to-properly-use-open-source-software-projects/', + ]; + yield 'no domain' => [null, 'https://shlink.io/documentation/']; + } } diff --git a/module/Rest/test-api/Action/EditShortUrlTagsActionTest.php b/module/Rest/test-api/Action/EditShortUrlTagsActionTest.php index d48ffc5f..0433a388 100644 --- a/module/Rest/test-api/Action/EditShortUrlTagsActionTest.php +++ b/module/Rest/test-api/Action/EditShortUrlTagsActionTest.php @@ -6,9 +6,12 @@ namespace ShlinkioApiTest\Shlink\Rest\Action; use GuzzleHttp\RequestOptions; use Shlinkio\Shlink\TestUtils\ApiTest\ApiTestCase; +use ShlinkioApiTest\Shlink\Rest\Utils\NotFoundUrlHelpersTrait; class EditShortUrlTagsActionTest extends ApiTestCase { + use NotFoundUrlHelpersTrait; + /** @test */ public function notProvidingTagsReturnsBadRequest(): void { @@ -24,12 +27,17 @@ class EditShortUrlTagsActionTest extends ApiTestCase $this->assertEquals('Invalid data', $payload['title']); } - /** @test */ - public function providingInvalidShortCodeReturnsBadRequest(): void - { - $expectedDetail = 'No URL found with short code "invalid"'; - - $resp = $this->callApiWithKey(self::METHOD_PUT, '/short-urls/invalid/tags', [RequestOptions::JSON => [ + /** + * @test + * @dataProvider provideInvalidUrls + */ + public function providingInvalidShortCodeReturnsBadRequest( + string $shortCode, + ?string $domain, + string $expectedDetail + ): void { + $url = $this->buildShortUrlPath($shortCode, $domain, '/tags'); + $resp = $this->callApiWithKey(self::METHOD_PUT, $url, [RequestOptions::JSON => [ 'tags' => ['foo', 'bar'], ]]); $payload = $this->getJsonResponsePayload($resp); @@ -39,6 +47,28 @@ class EditShortUrlTagsActionTest extends ApiTestCase $this->assertEquals('INVALID_SHORTCODE', $payload['type']); $this->assertEquals($expectedDetail, $payload['detail']); $this->assertEquals('Short URL not found', $payload['title']); - $this->assertEquals('invalid', $payload['shortCode']); + $this->assertEquals($shortCode, $payload['shortCode']); + $this->assertEquals($domain, $payload['domain'] ?? null); + } + + /** @test */ + public function tagsAreSetOnProperShortUrlBasedOnProvidedDomain(): void + { + $urlWithoutDomain = '/short-urls/ghi789/tags'; + $urlWithDomain = $urlWithoutDomain . '?domain=example.com'; + + $setTagsWithDomain = $this->callApiWithKey(self::METHOD_PUT, $urlWithDomain, [RequestOptions::JSON => [ + 'tags' => ['foo', 'bar'], + ]]); + $fetchWithoutDomain = $this->getJsonResponsePayload( + $this->callApiWithKey(self::METHOD_GET, '/short-urls/ghi789'), + ); + $fetchWithDomain = $this->getJsonResponsePayload( + $this->callApiWithKey(self::METHOD_GET, '/short-urls/ghi789?domain=example.com'), + ); + + $this->assertEquals(self::STATUS_OK, $setTagsWithDomain->getStatusCode()); + $this->assertEquals([], $fetchWithoutDomain['tags']); + $this->assertEquals(['bar', 'foo'], $fetchWithDomain['tags']); } } diff --git a/module/Rest/test-api/Action/GetVisitsActionTest.php b/module/Rest/test-api/Action/GetVisitsActionTest.php index f6167f78..cee466a3 100644 --- a/module/Rest/test-api/Action/GetVisitsActionTest.php +++ b/module/Rest/test-api/Action/GetVisitsActionTest.php @@ -4,16 +4,27 @@ declare(strict_types=1); namespace ShlinkioApiTest\Shlink\Rest\Action; +use Laminas\Diactoros\Uri; use Shlinkio\Shlink\TestUtils\ApiTest\ApiTestCase; +use ShlinkioApiTest\Shlink\Rest\Utils\NotFoundUrlHelpersTrait; + +use function GuzzleHttp\Psr7\build_query; +use function sprintf; class GetVisitsActionTest extends ApiTestCase { - /** @test */ - public function tryingToGetVisitsForInvalidUrlReturnsNotFoundError(): void - { - $expectedDetail = 'No URL found with short code "invalid"'; + use NotFoundUrlHelpersTrait; - $resp = $this->callApiWithKey(self::METHOD_GET, '/short-urls/invalid/visits'); + /** + * @test + * @dataProvider provideInvalidUrls + */ + public function tryingToGetVisitsForInvalidUrlReturnsNotFoundError( + string $shortCode, + ?string $domain, + string $expectedDetail + ): void { + $resp = $this->callApiWithKey(self::METHOD_GET, $this->buildShortUrlPath($shortCode, $domain, '/visits')); $payload = $this->getJsonResponsePayload($resp); $this->assertEquals(self::STATUS_NOT_FOUND, $resp->getStatusCode()); @@ -21,6 +32,33 @@ class GetVisitsActionTest extends ApiTestCase $this->assertEquals('INVALID_SHORTCODE', $payload['type']); $this->assertEquals($expectedDetail, $payload['detail']); $this->assertEquals('Short URL not found', $payload['title']); - $this->assertEquals('invalid', $payload['shortCode']); + $this->assertEquals($shortCode, $payload['shortCode']); + $this->assertEquals($domain, $payload['domain'] ?? null); + } + + /** + * @test + * @dataProvider provideDomains + */ + public function properVisitsAreReturnedWhenDomainIsProvided(?string $domain, int $expectedAmountOfVisits): void + { + $shortCode = 'ghi789'; + $url = new Uri(sprintf('/short-urls/%s/visits', $shortCode)); + + if ($domain !== null) { + $url = $url->withQuery(build_query(['domain' => $domain])); + } + + $resp = $this->callApiWithKey(self::METHOD_GET, (string) $url); + $payload = $this->getJsonResponsePayload($resp); + + $this->assertEquals($expectedAmountOfVisits, $payload['visits']['pagination']['totalItems'] ?? -1); + $this->assertCount($expectedAmountOfVisits, $payload['visits']['data'] ?? []); + } + + public function provideDomains(): iterable + { + yield 'domain' => ['example.com', 0]; + yield 'no domain' => [null, 2]; } } diff --git a/module/Rest/test-api/Action/ListShortUrlsTest.php b/module/Rest/test-api/Action/ListShortUrlsTest.php index f8d8c542..7d4e51a7 100644 --- a/module/Rest/test-api/Action/ListShortUrlsTest.php +++ b/module/Rest/test-api/Action/ListShortUrlsTest.php @@ -24,6 +24,21 @@ class ListShortUrlsTest extends ApiTestCase 'validUntil' => null, 'maxVisits' => null, ], + 'domain' => null, + ]; + private const SHORT_URL_DOCS = [ + 'shortCode' => 'ghi789', + 'shortUrl' => 'http://doma.in/ghi789', + 'longUrl' => 'https://shlink.io/documentation/', + 'dateCreated' => '2018-05-01T00:00:00+00:00', + 'visitsCount' => 2, + 'tags' => [], + 'meta' => [ + 'validSince' => null, + 'validUntil' => null, + 'maxVisits' => null, + ], + 'domain' => null, ]; private const SHORT_URL_CUSTOM_SLUG_AND_DOMAIN = [ 'shortCode' => 'custom-with-domain', @@ -37,6 +52,7 @@ class ListShortUrlsTest extends ApiTestCase 'validUntil' => null, 'maxVisits' => null, ], + 'domain' => 'some-domain.com', ]; private const SHORT_URL_META = [ 'shortCode' => 'def456', @@ -52,6 +68,7 @@ class ListShortUrlsTest extends ApiTestCase 'validUntil' => null, 'maxVisits' => null, ], + 'domain' => null, ]; private const SHORT_URL_CUSTOM_SLUG = [ 'shortCode' => 'custom', @@ -65,6 +82,7 @@ class ListShortUrlsTest extends ApiTestCase 'validUntil' => null, 'maxVisits' => 2, ], + 'domain' => null, ]; private const SHORT_URL_CUSTOM_DOMAIN = [ 'shortCode' => 'ghi789', @@ -80,6 +98,7 @@ class ListShortUrlsTest extends ApiTestCase 'validUntil' => null, 'maxVisits' => null, ], + 'domain' => 'example.com', ]; /** @@ -104,6 +123,7 @@ class ListShortUrlsTest extends ApiTestCase { yield [[], [ self::SHORT_URL_SHLINK, + self::SHORT_URL_DOCS, self::SHORT_URL_CUSTOM_SLUG_AND_DOMAIN, self::SHORT_URL_META, self::SHORT_URL_CUSTOM_SLUG, @@ -114,8 +134,17 @@ class ListShortUrlsTest extends ApiTestCase self::SHORT_URL_CUSTOM_SLUG, self::SHORT_URL_CUSTOM_SLUG_AND_DOMAIN, self::SHORT_URL_META, + self::SHORT_URL_DOCS, self::SHORT_URL_CUSTOM_DOMAIN, ]]; + yield [['orderBy' => ['shortCode' => 'DESC']], [ + self::SHORT_URL_DOCS, + self::SHORT_URL_CUSTOM_DOMAIN, + self::SHORT_URL_META, + self::SHORT_URL_CUSTOM_SLUG_AND_DOMAIN, + self::SHORT_URL_CUSTOM_SLUG, + self::SHORT_URL_SHLINK, + ]]; yield [['startDate' => Chronos::parse('2018-12-01')->toAtomString()], [ self::SHORT_URL_META, self::SHORT_URL_CUSTOM_SLUG, @@ -123,6 +152,7 @@ class ListShortUrlsTest extends ApiTestCase ]]; yield [['endDate' => Chronos::parse('2018-12-01')->toAtomString()], [ self::SHORT_URL_SHLINK, + self::SHORT_URL_DOCS, self::SHORT_URL_CUSTOM_SLUG_AND_DOMAIN, ]]; yield [['tags' => ['foo']], [ @@ -139,6 +169,9 @@ class ListShortUrlsTest extends ApiTestCase self::SHORT_URL_META, self::SHORT_URL_CUSTOM_DOMAIN, ]]; + yield [['searchTerm' => 'example.com'], [ + self::SHORT_URL_CUSTOM_DOMAIN, + ]]; } private function buildPagination(int $itemsCount): array diff --git a/module/Rest/test-api/Action/ResolveShortUrlActionTest.php b/module/Rest/test-api/Action/ResolveShortUrlActionTest.php index 27d9dd69..d76d7946 100644 --- a/module/Rest/test-api/Action/ResolveShortUrlActionTest.php +++ b/module/Rest/test-api/Action/ResolveShortUrlActionTest.php @@ -7,11 +7,14 @@ namespace ShlinkioApiTest\Shlink\Rest\Action; use Cake\Chronos\Chronos; use GuzzleHttp\RequestOptions; use Shlinkio\Shlink\TestUtils\ApiTest\ApiTestCase; +use ShlinkioApiTest\Shlink\Rest\Utils\NotFoundUrlHelpersTrait; use function sprintf; class ResolveShortUrlActionTest extends ApiTestCase { + use NotFoundUrlHelpersTrait; + /** * @test * @dataProvider provideDisabledMeta @@ -40,12 +43,16 @@ class ResolveShortUrlActionTest extends ApiTestCase yield 'maxVisits reached' => [['maxVisits' => 1]]; } - /** @test */ - public function tryingToResolveInvalidUrlReturnsNotFoundError(): void - { - $expectedDetail = 'No URL found with short code "invalid"'; - - $resp = $this->callApiWithKey(self::METHOD_GET, '/short-urls/invalid'); + /** + * @test + * @dataProvider provideInvalidUrls + */ + public function tryingToResolveInvalidUrlReturnsNotFoundError( + string $shortCode, + ?string $domain, + string $expectedDetail + ): void { + $resp = $this->callApiWithKey(self::METHOD_GET, $this->buildShortUrlPath($shortCode, $domain)); $payload = $this->getJsonResponsePayload($resp); $this->assertEquals(self::STATUS_NOT_FOUND, $resp->getStatusCode()); @@ -53,6 +60,7 @@ class ResolveShortUrlActionTest extends ApiTestCase $this->assertEquals('INVALID_SHORTCODE', $payload['type']); $this->assertEquals($expectedDetail, $payload['detail']); $this->assertEquals('Short URL not found', $payload['title']); - $this->assertEquals('invalid', $payload['shortCode']); + $this->assertEquals($shortCode, $payload['shortCode']); + $this->assertEquals($domain, $payload['domain'] ?? null); } } diff --git a/module/Rest/test-api/Fixtures/ShortUrlsFixture.php b/module/Rest/test-api/Fixtures/ShortUrlsFixture.php index d7566063..0aa13a82 100644 --- a/module/Rest/test-api/Fixtures/ShortUrlsFixture.php +++ b/module/Rest/test-api/Fixtures/ShortUrlsFixture.php @@ -37,11 +37,17 @@ class ShortUrlsFixture extends AbstractFixture ), '2019-01-01 00:00:20'); $manager->persist($customShortUrl); - $withDomainShortUrl = $this->setShortUrlDate(new ShortUrl( + $ghiShortUrl = $this->setShortUrlDate( + new ShortUrl('https://shlink.io/documentation/', ShortUrlMeta::fromRawData(['customSlug' => 'ghi789'])), + '2018-05-01', + ); + $manager->persist($ghiShortUrl); + + $withDomainDuplicatingShortCode = $this->setShortUrlDate(new ShortUrl( 'https://blog.alejandrocelaya.com/2019/04/27/considerations-to-properly-use-open-source-software-projects/', ShortUrlMeta::fromRawData(['domain' => 'example.com', 'customSlug' => 'ghi789']), ), '2019-01-01 00:00:30'); - $manager->persist($withDomainShortUrl); + $manager->persist($withDomainDuplicatingShortCode); $withDomainAndSlugShortUrl = $this->setShortUrlDate(new ShortUrl( 'https://google.com', @@ -53,6 +59,7 @@ class ShortUrlsFixture extends AbstractFixture $this->addReference('abc123_short_url', $abcShortUrl); $this->addReference('def456_short_url', $defShortUrl); + $this->addReference('ghi789_short_url', $ghiShortUrl); } private function setShortUrlDate(ShortUrl $shortUrl, string $date): ShortUrl diff --git a/module/Rest/test-api/Fixtures/VisitsFixture.php b/module/Rest/test-api/Fixtures/VisitsFixture.php index 2c85c1a1..a07d95d1 100644 --- a/module/Rest/test-api/Fixtures/VisitsFixture.php +++ b/module/Rest/test-api/Fixtures/VisitsFixture.php @@ -31,6 +31,11 @@ class VisitsFixture extends AbstractFixture implements DependentFixtureInterface $manager->persist(new Visit($defShortUrl, new Visitor('shlink-tests-agent', '', '127.0.0.1'))); $manager->persist(new Visit($defShortUrl, new Visitor('shlink-tests-agent', 'https://app.shlink.io', ''))); + /** @var ShortUrl $defShortUrl */ + $defShortUrl = $this->getReference('ghi789_short_url'); + $manager->persist(new Visit($defShortUrl, new Visitor('shlink-tests-agent', '', '1.2.3.4'))); + $manager->persist(new Visit($defShortUrl, new Visitor('shlink-tests-agent', 'https://app.shlink.io', ''))); + $manager->flush(); } } diff --git a/module/Rest/test-api/Utils/NotFoundUrlHelpersTrait.php b/module/Rest/test-api/Utils/NotFoundUrlHelpersTrait.php new file mode 100644 index 00000000..3cf2ad30 --- /dev/null +++ b/module/Rest/test-api/Utils/NotFoundUrlHelpersTrait.php @@ -0,0 +1,38 @@ + ['invalid', null, 'No URL found with short code "invalid"']; + yield 'invalid shortcode without domain' => [ + 'abc123', + 'example.com', + 'No URL found with short code "abc123" for domain "example.com"', + ]; + yield 'invalid shortcode + domain' => [ + 'custom-with-domain', + 'example.com', + 'No URL found with short code "custom-with-domain" for domain "example.com"', + ]; + } + + public function buildShortUrlPath(string $shortCode, ?string $domain, string $suffix = ''): string + { + $url = new Uri(sprintf('/short-urls/%s%s', $shortCode, $suffix)); + if ($domain !== null) { + $url = $url->withQuery(build_query(['domain' => $domain])); + } + + return (string) $url; + } +} diff --git a/module/Rest/test/Action/ShortUrl/EditShortUrlTagsActionTest.php b/module/Rest/test/Action/ShortUrl/EditShortUrlTagsActionTest.php index 83db484c..d7a86844 100644 --- a/module/Rest/test/Action/ShortUrl/EditShortUrlTagsActionTest.php +++ b/module/Rest/test/Action/ShortUrl/EditShortUrlTagsActionTest.php @@ -9,6 +9,7 @@ use PHPUnit\Framework\TestCase; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Exception\ValidationException; +use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Service\ShortUrlService; use Shlinkio\Shlink\Rest\Action\ShortUrl\EditShortUrlTagsAction; @@ -34,8 +35,8 @@ class EditShortUrlTagsActionTest extends TestCase public function tagsListIsReturnedIfCorrectShortCodeIsProvided(): void { $shortCode = 'abc123'; - $this->shortUrlService->setTagsByShortCode($shortCode, [])->willReturn(new ShortUrl('')) - ->shouldBeCalledOnce(); + $this->shortUrlService->setTagsByShortCode(new ShortUrlIdentifier($shortCode), [])->willReturn(new ShortUrl('')) + ->shouldBeCalledOnce(); $response = $this->action->handle( (new ServerRequest())->withAttribute('shortCode', 'abc123') diff --git a/module/Rest/test/Action/ShortUrl/ListShortUrlsActionTest.php b/module/Rest/test/Action/ShortUrl/ListShortUrlsActionTest.php index 52cc62de..3d98c2fe 100644 --- a/module/Rest/test/Action/ShortUrl/ListShortUrlsActionTest.php +++ b/module/Rest/test/Action/ShortUrl/ListShortUrlsActionTest.php @@ -12,7 +12,7 @@ use Laminas\Paginator\Paginator; use PHPUnit\Framework\TestCase; use Prophecy\Prophecy\ObjectProphecy; use Psr\Log\LoggerInterface; -use Shlinkio\Shlink\Common\Util\DateRange; +use Shlinkio\Shlink\Core\Model\ShortUrlsParams; use Shlinkio\Shlink\Core\Service\ShortUrlService; use Shlinkio\Shlink\Rest\Action\ShortUrl\ListShortUrlsAction; @@ -43,15 +43,17 @@ class ListShortUrlsActionTest extends TestCase ?string $expectedSearchTerm, array $expectedTags, ?string $expectedOrderBy, - DateRange $expectedDateRange + ?string $startDate = null, + ?string $endDate = null ): void { - $listShortUrls = $this->service->listShortUrls( - $expectedPage, - $expectedSearchTerm, - $expectedTags, - $expectedOrderBy, - $expectedDateRange, - )->willReturn(new Paginator(new ArrayAdapter())); + $listShortUrls = $this->service->listShortUrls(ShortUrlsParams::fromRawData([ + 'page' => $expectedPage, + 'searchTerm' => $expectedSearchTerm, + 'tags' => $expectedTags, + 'orderBy' => $expectedOrderBy, + 'startDate' => $startDate, + 'endDate' => $endDate, + ]))->willReturn(new Paginator(new ArrayAdapter())); /** @var JsonResponse $response */ $response = $this->action->handle((new ServerRequest())->withQueryParams($query)); @@ -66,25 +68,25 @@ class ListShortUrlsActionTest extends TestCase public function provideFilteringData(): iterable { - yield [[], 1, null, [], null, new DateRange()]; - yield [['page' => 10], 10, null, [], null, new DateRange()]; - yield [['page' => null], 1, null, [], null, new DateRange()]; - yield [['page' => '8'], 8, null, [], null, new DateRange()]; - yield [['searchTerm' => $searchTerm = 'foo'], 1, $searchTerm, [], null, new DateRange()]; - yield [['tags' => $tags = ['foo','bar']], 1, null, $tags, null, new DateRange()]; - yield [['orderBy' => $orderBy = 'something'], 1, null, [], $orderBy, new DateRange()]; + yield [[], 1, null, [], null]; + yield [['page' => 10], 10, null, [], null]; + yield [['page' => null], 1, null, [], null]; + yield [['page' => '8'], 8, null, [], null]; + yield [['searchTerm' => $searchTerm = 'foo'], 1, $searchTerm, [], null]; + yield [['tags' => $tags = ['foo','bar']], 1, null, $tags, null]; + yield [['orderBy' => $orderBy = 'something'], 1, null, [], $orderBy]; yield [[ 'page' => '2', 'orderBy' => $orderBy = 'something', 'tags' => $tags = ['one', 'two'], - ], 2, null, $tags, $orderBy, new DateRange()]; + ], 2, null, $tags, $orderBy]; yield [ ['startDate' => $date = Chronos::now()->toAtomString()], 1, null, [], null, - new DateRange(Chronos::parse($date)), + $date, ]; yield [ ['endDate' => $date = Chronos::now()->toAtomString()], @@ -92,7 +94,8 @@ class ListShortUrlsActionTest extends TestCase null, [], null, - new DateRange(null, Chronos::parse($date)), + null, + $date, ]; yield [ [ @@ -103,7 +106,8 @@ class ListShortUrlsActionTest extends TestCase null, [], null, - new DateRange(Chronos::parse($startDate), Chronos::parse($endDate)), + $startDate, + $endDate, ]; } } diff --git a/module/Rest/test/Action/ShortUrl/ResolveShortUrlActionTest.php b/module/Rest/test/Action/ShortUrl/ResolveShortUrlActionTest.php index 067b4e0e..a62b1f95 100644 --- a/module/Rest/test/Action/ShortUrl/ResolveShortUrlActionTest.php +++ b/module/Rest/test/Action/ShortUrl/ResolveShortUrlActionTest.php @@ -8,6 +8,7 @@ use Laminas\Diactoros\ServerRequest; use PHPUnit\Framework\TestCase; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\Core\Entity\ShortUrl; +use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Service\ShortUrl\ShortUrlResolverInterface; use Shlinkio\Shlink\Rest\Action\ShortUrl\ResolveShortUrlAction; @@ -28,7 +29,7 @@ class ResolveShortUrlActionTest extends TestCase public function correctShortCodeReturnsSuccess(): void { $shortCode = 'abc123'; - $this->urlResolver->shortCodeToShortUrl($shortCode, null)->willReturn( + $this->urlResolver->resolveShortUrl(new ShortUrlIdentifier($shortCode))->willReturn( new ShortUrl('http://domain.com/foo/bar'), )->shouldBeCalledOnce(); diff --git a/module/Rest/test/Action/Visit/GetVisitsActionTest.php b/module/Rest/test/Action/Visit/GetVisitsActionTest.php index a445c3b2..a1f1681a 100644 --- a/module/Rest/test/Action/Visit/GetVisitsActionTest.php +++ b/module/Rest/test/Action/Visit/GetVisitsActionTest.php @@ -12,6 +12,7 @@ use PHPUnit\Framework\TestCase; use Prophecy\Argument; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\Common\Util\DateRange; +use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Model\VisitsParams; use Shlinkio\Shlink\Core\Service\VisitsTracker; use Shlinkio\Shlink\Rest\Action\Visit\GetVisitsAction; @@ -31,7 +32,7 @@ class GetVisitsActionTest extends TestCase public function providingCorrectShortCodeReturnsVisits(): void { $shortCode = 'abc123'; - $this->visitsTracker->info($shortCode, Argument::type(VisitsParams::class))->willReturn( + $this->visitsTracker->info(new ShortUrlIdentifier($shortCode), Argument::type(VisitsParams::class))->willReturn( new Paginator(new ArrayAdapter([])), )->shouldBeCalledOnce(); @@ -43,7 +44,7 @@ class GetVisitsActionTest extends TestCase public function paramsAreReadFromQuery(): void { $shortCode = 'abc123'; - $this->visitsTracker->info($shortCode, new VisitsParams( + $this->visitsTracker->info(new ShortUrlIdentifier($shortCode), new VisitsParams( new DateRange(null, Chronos::parse('2016-01-01 00:00:00')), 3, 10, diff --git a/module/Rest/test/Middleware/ShortUrl/DropDefaultDomainFromQueryMiddlewareTest.php b/module/Rest/test/Middleware/ShortUrl/DropDefaultDomainFromQueryMiddlewareTest.php new file mode 100644 index 00000000..8f588304 --- /dev/null +++ b/module/Rest/test/Middleware/ShortUrl/DropDefaultDomainFromQueryMiddlewareTest.php @@ -0,0 +1,54 @@ +next = $this->prophesize(RequestHandlerInterface::class); + $this->middleware = new DropDefaultDomainFromQueryMiddleware('doma.in'); + } + + /** + * @test + * @dataProvider provideQueryParams + */ + public function domainIsDroppedWhenDefaultOneIsProvided(array $providedQuery, array $expectedQuery): void + { + $req = ServerRequestFactory::fromGlobals()->withQueryParams($providedQuery); + + $handle = $this->next->handle(Argument::that(function (ServerRequestInterface $request) use ($expectedQuery) { + Assert::assertEquals($expectedQuery, $request->getQueryParams()); + return $request; + }))->willReturn(new Response()); + + $this->middleware->process($req, $this->next->reveal()); + + $handle->shouldHaveBeenCalledOnce(); + } + + public function provideQueryParams(): iterable + { + yield [[], []]; + yield [['foo' => 'bar'], ['foo' => 'bar']]; + yield [['foo' => 'bar', 'domain' => 'doma.in'], ['foo' => 'bar']]; + yield [['foo' => 'bar', 'domain' => 'not_default'], ['foo' => 'bar', 'domain' => 'not_default']]; + yield [['domain' => 'doma.in'], []]; + } +} diff --git a/phpstan.neon b/phpstan.neon index b6c65f7f..d983a985 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -2,6 +2,5 @@ parameters: checkMissingIterableValueType: false checkGenericClassInNonGenericObjectType: false ignoreErrors: - - '#Undefined variable: \$metadata#' - '#AbstractQuery::setParameters()#' - '#mustRun()#'