diff --git a/CHANGELOG.md b/CHANGELOG.md index 9fe5ac60..7bdbbabf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,7 +13,6 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this #### Changed * [#577](https://github.com/shlinkio/shlink/issues/577) Wrapped params used to customize short URL lists into a DTO with implicit validation. -* [#620](https://github.com/shlinkio/shlink/issues/620) "Controlled" errors (like validation errors and such) will no longer be logged with error level, preventing logs to be polluted. #### Deprecated @@ -25,7 +24,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this #### Fixed -* *Nothing* +* [#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. ## 2.0.3 - 2020-01-27 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/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/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/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/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 cd96acfc..a9d21952 100644 --- a/module/Core/src/Repository/ShortUrlRepository.php +++ b/module/Core/src/Repository/ShortUrlRepository.php @@ -90,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')); @@ -127,7 +127,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 @@ -159,14 +159,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') @@ -176,7 +192,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 d0acdf1a..065198b4 100644 --- a/module/Core/src/Repository/ShortUrlRepositoryInterface.php +++ b/module/Core/src/Repository/ShortUrlRepositoryInterface.php @@ -22,7 +22,9 @@ interface ShortUrlRepositoryInterface extends ObjectRepository 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 dbfb1db6..e9aaf637 100644 --- a/module/Core/src/Service/ShortUrlService.php +++ b/module/Core/src/Service/ShortUrlService.php @@ -8,23 +8,25 @@ use Doctrine\ORM; use Laminas\Paginator\Paginator; 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; } /** @@ -45,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; @@ -57,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 e431c51b..379abc55 100644 --- a/module/Core/src/Service/ShortUrlServiceInterface.php +++ b/module/Core/src/Service/ShortUrlServiceInterface.php @@ -7,6 +7,7 @@ namespace Shlinkio\Shlink\Core\Service; use Laminas\Paginator\Paginator; 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; @@ -21,10 +22,10 @@ interface ShortUrlServiceInterface * @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/test-db/Repository/ShortUrlRepositoryTest.php b/module/Core/test-db/Repository/ShortUrlRepositoryTest.php index 4af2fd61..4829fada 100644 --- a/module/Core/test-db/Repository/ShortUrlRepositoryTest.php +++ b/module/Core/test-db/Repository/ShortUrlRepositoryTest.php @@ -37,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); @@ -54,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 */ @@ -183,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/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 69f0785d..842eac60 100644 --- a/module/Core/test/Service/ShortUrlServiceTest.php +++ b/module/Core/test/Service/ShortUrlServiceTest.php @@ -12,10 +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; @@ -24,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 */ @@ -52,36 +57,21 @@ class ShortUrlServiceTest extends TestCase $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 */ @@ -89,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/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/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 3fec6f7a..7af30948 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,9 +134,11 @@ 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, @@ -130,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']], [ 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/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'], []]; + } +}