diff --git a/module/CLI/config/dependencies.config.php b/module/CLI/config/dependencies.config.php index dfa8452f..1f94f5a6 100644 --- a/module/CLI/config/dependencies.config.php +++ b/module/CLI/config/dependencies.config.php @@ -55,7 +55,7 @@ return [ GeolocationDbUpdater::class => [DbUpdater::class, Reader::class, 'Shlinkio\Shlink\LocalLockFactory'], Command\ShortUrl\GenerateShortUrlCommand::class => [Service\UrlShortener::class, 'config.url_shortener.domain'], - Command\ShortUrl\ResolveUrlCommand::class => [Service\UrlShortener::class], + Command\ShortUrl\ResolveUrlCommand::class => [Service\ShortUrl\ShortUrlResolver::class], Command\ShortUrl\ListShortUrlsCommand::class => [Service\ShortUrlService::class, 'config.url_shortener.domain'], Command\ShortUrl\GetVisitsCommand::class => [Service\VisitsTracker::class], Command\ShortUrl\DeleteShortUrlCommand::class => [Service\ShortUrl\DeleteShortUrlService::class], diff --git a/module/CLI/src/Command/ShortUrl/ResolveUrlCommand.php b/module/CLI/src/Command/ShortUrl/ResolveUrlCommand.php index e4c75410..22f384db 100644 --- a/module/CLI/src/Command/ShortUrl/ResolveUrlCommand.php +++ b/module/CLI/src/Command/ShortUrl/ResolveUrlCommand.php @@ -6,7 +6,7 @@ namespace Shlinkio\Shlink\CLI\Command\ShortUrl; use Shlinkio\Shlink\CLI\Util\ExitCodes; use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; -use Shlinkio\Shlink\Core\Service\UrlShortenerInterface; +use Shlinkio\Shlink\Core\Service\ShortUrl\ShortUrlResolverInterface; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; @@ -20,12 +20,12 @@ class ResolveUrlCommand extends Command { public const NAME = 'short-url:parse'; - private UrlShortenerInterface $urlShortener; + private ShortUrlResolverInterface $urlResolver; - public function __construct(UrlShortenerInterface $urlShortener) + public function __construct(ShortUrlResolverInterface $urlResolver) { parent::__construct(); - $this->urlShortener = $urlShortener; + $this->urlResolver = $urlResolver; } protected function configure(): void @@ -58,7 +58,7 @@ class ResolveUrlCommand extends Command $domain = $input->getOption('domain'); try { - $url = $this->urlShortener->shortCodeToUrl($shortCode, $domain); + $url = $this->urlResolver->shortCodeToShortUrl($shortCode, $domain); $output->writeln(sprintf('Long URL: %s', $url->getLongUrl())); return ExitCodes::EXIT_SUCCESS; } catch (ShortUrlNotFoundException $e) { diff --git a/module/CLI/test/Command/ShortUrl/ResolveUrlCommandTest.php b/module/CLI/test/Command/ShortUrl/ResolveUrlCommandTest.php index 1fe8b238..cb3e658d 100644 --- a/module/CLI/test/Command/ShortUrl/ResolveUrlCommandTest.php +++ b/module/CLI/test/Command/ShortUrl/ResolveUrlCommandTest.php @@ -9,7 +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\Service\UrlShortener; +use Shlinkio\Shlink\Core\Service\ShortUrl\ShortUrlResolverInterface; use Symfony\Component\Console\Application; use Symfony\Component\Console\Tester\CommandTester; @@ -20,12 +20,12 @@ use const PHP_EOL; class ResolveUrlCommandTest extends TestCase { private CommandTester $commandTester; - private ObjectProphecy $urlShortener; + private ObjectProphecy $urlResolver; public function setUp(): void { - $this->urlShortener = $this->prophesize(UrlShortener::class); - $command = new ResolveUrlCommand($this->urlShortener->reveal()); + $this->urlResolver = $this->prophesize(ShortUrlResolverInterface::class); + $command = new ResolveUrlCommand($this->urlResolver->reveal()); $app = new Application(); $app->add($command); @@ -38,8 +38,8 @@ class ResolveUrlCommandTest extends TestCase $shortCode = 'abc123'; $expectedUrl = 'http://domain.com/foo/bar'; $shortUrl = new ShortUrl($expectedUrl); - $this->urlShortener->shortCodeToUrl($shortCode, null)->willReturn($shortUrl) - ->shouldBeCalledOnce(); + $this->urlResolver->shortCodeToShortUrl($shortCode, null)->willReturn($shortUrl) + ->shouldBeCalledOnce(); $this->commandTester->execute(['shortCode' => $shortCode]); $output = $this->commandTester->getDisplay(); @@ -50,7 +50,7 @@ class ResolveUrlCommandTest extends TestCase public function incorrectShortCodeOutputsErrorMessage(): void { $shortCode = 'abc123'; - $this->urlShortener->shortCodeToUrl($shortCode, null) + $this->urlResolver->shortCodeToShortUrl($shortCode, null) ->willThrow(ShortUrlNotFoundException::fromNotFoundShortCode($shortCode)) ->shouldBeCalledOnce(); diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index 8d053c4f..e1954329 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -30,6 +30,7 @@ return [ Service\VisitService::class => ConfigAbstractFactory::class, Service\Tag\TagService::class => ConfigAbstractFactory::class, Service\ShortUrl\DeleteShortUrlService::class => ConfigAbstractFactory::class, + Service\ShortUrl\ShortUrlResolver::class => ConfigAbstractFactory::class, Util\UrlValidator::class => ConfigAbstractFactory::class, @@ -56,22 +57,27 @@ return [ Service\VisitService::class => ['em'], Service\Tag\TagService::class => ['em'], Service\ShortUrl\DeleteShortUrlService::class => ['em', Options\DeleteShortUrlsOptions::class], + Service\ShortUrl\ShortUrlResolver::class => ['em'], Util\UrlValidator::class => ['httpClient'], Action\RedirectAction::class => [ - Service\UrlShortener::class, + Service\ShortUrl\ShortUrlResolver::class, Service\VisitsTracker::class, Options\AppOptions::class, 'Logger_Shlink', ], Action\PixelAction::class => [ - Service\UrlShortener::class, + Service\ShortUrl\ShortUrlResolver::class, Service\VisitsTracker::class, Options\AppOptions::class, 'Logger_Shlink', ], - Action\QrCodeAction::class => [RouterInterface::class, Service\UrlShortener::class, 'Logger_Shlink'], + Action\QrCodeAction::class => [ + RouterInterface::class, + Service\ShortUrl\ShortUrlResolver::class, + 'Logger_Shlink', + ], Middleware\QrCodeCacheMiddleware::class => [Cache::class], ], diff --git a/module/Core/src/Action/AbstractTrackingAction.php b/module/Core/src/Action/AbstractTrackingAction.php index 8cac42fc..4e45d9cd 100644 --- a/module/Core/src/Action/AbstractTrackingAction.php +++ b/module/Core/src/Action/AbstractTrackingAction.php @@ -15,7 +15,7 @@ use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; use Shlinkio\Shlink\Core\Model\Visitor; use Shlinkio\Shlink\Core\Options\AppOptions; -use Shlinkio\Shlink\Core\Service\UrlShortenerInterface; +use Shlinkio\Shlink\Core\Service\ShortUrl\ShortUrlResolverInterface; use Shlinkio\Shlink\Core\Service\VisitsTrackerInterface; use function array_key_exists; @@ -25,18 +25,18 @@ use function GuzzleHttp\Psr7\parse_query; abstract class AbstractTrackingAction implements MiddlewareInterface { - private UrlShortenerInterface $urlShortener; + private ShortUrlResolverInterface $urlResolver; private VisitsTrackerInterface $visitTracker; private AppOptions $appOptions; private LoggerInterface $logger; public function __construct( - UrlShortenerInterface $urlShortener, + ShortUrlResolverInterface $urlResolver, VisitsTrackerInterface $visitTracker, AppOptions $appOptions, ?LoggerInterface $logger = null ) { - $this->urlShortener = $urlShortener; + $this->urlResolver = $urlResolver; $this->visitTracker = $visitTracker; $this->appOptions = $appOptions; $this->logger = $logger ?: new NullLogger(); @@ -50,7 +50,7 @@ abstract class AbstractTrackingAction implements MiddlewareInterface $disableTrackParam = $this->appOptions->getDisableTrackParam(); try { - $url = $this->urlShortener->shortCodeToUrl($shortCode, $domain); + $url = $this->urlResolver->shortCodeToEnabledShortUrl($shortCode, $domain); // Track visit to this short code if ($disableTrackParam === null || ! array_key_exists($disableTrackParam, $query)) { diff --git a/module/Core/src/Action/QrCodeAction.php b/module/Core/src/Action/QrCodeAction.php index a1a58ae7..c302a58d 100644 --- a/module/Core/src/Action/QrCodeAction.php +++ b/module/Core/src/Action/QrCodeAction.php @@ -5,7 +5,6 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Action; use Endroid\QrCode\QrCode; -use Mezzio\Router\Exception\RuntimeException; use Mezzio\Router\RouterInterface; use Psr\Http\Message\ResponseInterface as Response; use Psr\Http\Message\ServerRequestInterface as Request; @@ -15,7 +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\Service\UrlShortenerInterface; +use Shlinkio\Shlink\Core\Service\ShortUrl\ShortUrlResolverInterface; class QrCodeAction implements MiddlewareInterface { @@ -24,27 +23,19 @@ class QrCodeAction implements MiddlewareInterface private const MAX_SIZE = 1000; private RouterInterface $router; - private UrlShortenerInterface $urlShortener; + private ShortUrlResolverInterface $urlResolver; private LoggerInterface $logger; public function __construct( RouterInterface $router, - UrlShortenerInterface $urlShortener, + ShortUrlResolverInterface $urlResolver, ?LoggerInterface $logger = null ) { $this->router = $router; - $this->urlShortener = $urlShortener; + $this->urlResolver = $urlResolver; $this->logger = $logger ?: new NullLogger(); } - /** - * Process an incoming server request and return a response, optionally delegating - * to the next middleware component to create the response. - * - * - * @throws \InvalidArgumentException - * @throws RuntimeException - */ public function process(Request $request, RequestHandlerInterface $handler): Response { // Make sure the short URL exists for this short code @@ -52,7 +43,7 @@ class QrCodeAction implements MiddlewareInterface $domain = $request->getUri()->getAuthority(); try { - $this->urlShortener->shortCodeToUrl($shortCode, $domain); + $this->urlResolver->shortCodeToEnabledShortUrl($shortCode, $domain); } catch (ShortUrlNotFoundException $e) { $this->logger->warning('An error occurred while creating QR code. {e}', ['e' => $e]); return $handler->handle($request); diff --git a/module/Core/src/Entity/ShortUrl.php b/module/Core/src/Entity/ShortUrl.php index 2e14bb3f..bff5549c 100644 --- a/module/Core/src/Entity/ShortUrl.php +++ b/module/Core/src/Entity/ShortUrl.php @@ -149,9 +149,25 @@ class ShortUrl extends AbstractEntity return $this->maxVisits; } - public function maxVisitsReached(): bool + public function isEnabled(): bool { - return $this->maxVisits !== null && $this->getVisitsCount() >= $this->maxVisits; + $maxVisitsReached = $this->maxVisits !== null && $this->getVisitsCount() >= $this->maxVisits; + if ($maxVisitsReached) { + return false; + } + + $now = Chronos::now(); + $beforeValidSince = $this->validSince !== null && $this->validSince->gt($now); + if ($beforeValidSince) { + return false; + } + + $afterValidUntil = $this->validUntil !== null && $this->validUntil->lt($now); + if ($afterValidUntil) { + return false; + } + + return true; } public function toString(array $domainConfig): string @@ -186,12 +202,10 @@ class ShortUrl extends AbstractEntity } $shortUrlTags = invoke($this->getTags(), '__toString'); - $hasAllTags = count($shortUrlTags) === count($tags) && array_reduce( + return count($shortUrlTags) === count($tags) && array_reduce( $tags, fn (bool $hasAllTags, string $tag) => $hasAllTags && contains($shortUrlTags, $tag), true, ); - - return $hasAllTags; } } diff --git a/module/Core/src/Repository/ShortUrlRepository.php b/module/Core/src/Repository/ShortUrlRepository.php index dafc841f..befa1b40 100644 --- a/module/Core/src/Repository/ShortUrlRepository.php +++ b/module/Core/src/Repository/ShortUrlRepository.php @@ -146,8 +146,6 @@ class ShortUrlRepository extends EntityRepository implements ShortUrlRepositoryI FROM Shlinkio\Shlink\Core\Entity\ShortUrl AS s LEFT JOIN s.domain AS d WHERE s.shortCode = :shortCode - AND (s.validSince <= :now OR s.validSince IS NULL) - AND (s.validUntil >= :now OR s.validUntil IS NULL) AND (s.domain IS NULL OR d.authority = :domain) ORDER BY s.domain {$ordering} DQL; @@ -156,7 +154,6 @@ DQL; $query->setMaxResults(1) ->setParameters([ 'shortCode' => $shortCode, - 'now' => Chronos::now(), 'domain' => $domain, ]); @@ -166,9 +163,7 @@ DQL; // * The short URL matching the short code but without any domain, or // * No short URL at all - /** @var ShortUrl|null $shortUrl */ - $shortUrl = $query->getOneOrNullResult(); - return $shortUrl !== null && ! $shortUrl->maxVisitsReached() ? $shortUrl : null; + return $query->getOneOrNullResult(); } public function shortCodeIsInUse(string $slug, ?string $domain = null): bool diff --git a/module/Core/src/Service/ShortUrl/ShortUrlResolver.php b/module/Core/src/Service/ShortUrl/ShortUrlResolver.php new file mode 100644 index 00000000..62f30c11 --- /dev/null +++ b/module/Core/src/Service/ShortUrl/ShortUrlResolver.php @@ -0,0 +1,48 @@ +em = $em; + } + + /** + * @throws ShortUrlNotFoundException + */ + public function shortCodeToShortUrl(string $shortCode, ?string $domain = null): ShortUrl + { + /** @var ShortUrlRepository $shortUrlRepo */ + $shortUrlRepo = $this->em->getRepository(ShortUrl::class); + $shortUrl = $shortUrlRepo->findOneByShortCode($shortCode, $domain); + if ($shortUrl === null) { + throw ShortUrlNotFoundException::fromNotFoundShortCode($shortCode, $domain); + } + + return $shortUrl; + } + + /** + * @throws ShortUrlNotFoundException + */ + public function shortCodeToEnabledShortUrl(string $shortCode, ?string $domain = null): ShortUrl + { + $shortUrl = $this->shortCodeToShortUrl($shortCode, $domain); + if (! $shortUrl->isEnabled()) { + throw ShortUrlNotFoundException::fromNotFoundShortCode($shortCode, $domain); + } + + return $shortUrl; + } +} diff --git a/module/Core/src/Service/ShortUrl/ShortUrlResolverInterface.php b/module/Core/src/Service/ShortUrl/ShortUrlResolverInterface.php new file mode 100644 index 00000000..b00beed5 --- /dev/null +++ b/module/Core/src/Service/ShortUrl/ShortUrlResolverInterface.php @@ -0,0 +1,21 @@ +urlShortener = $this->prophesize(UrlShortener::class); + $this->urlResolver = $this->prophesize(ShortUrlResolverInterface::class); $this->visitTracker = $this->prophesize(VisitsTracker::class); $this->action = new PixelAction( - $this->urlShortener->reveal(), + $this->urlResolver->reveal(), $this->visitTracker->reveal(), new AppOptions(), ); @@ -38,7 +38,7 @@ class PixelActionTest extends TestCase public function imageIsReturned(): void { $shortCode = 'abc123'; - $this->urlShortener->shortCodeToUrl($shortCode, '')->willReturn( + $this->urlResolver->shortCodeToEnabledShortUrl($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 e98f0393..63df94af 100644 --- a/module/Core/test/Action/QrCodeActionTest.php +++ b/module/Core/test/Action/QrCodeActionTest.php @@ -15,29 +15,29 @@ 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\Service\UrlShortener; +use Shlinkio\Shlink\Core\Service\ShortUrl\ShortUrlResolverInterface; class QrCodeActionTest extends TestCase { private QrCodeAction $action; - private ObjectProphecy $urlShortener; + private ObjectProphecy $urlResolver; public function setUp(): void { $router = $this->prophesize(RouterInterface::class); $router->generateUri(Argument::cetera())->willReturn('/foo/bar'); - $this->urlShortener = $this->prophesize(UrlShortener::class); + $this->urlResolver = $this->prophesize(ShortUrlResolverInterface::class); - $this->action = new QrCodeAction($router->reveal(), $this->urlShortener->reveal()); + $this->action = new QrCodeAction($router->reveal(), $this->urlResolver->reveal()); } /** @test */ public function aNotFoundShortCodeWillDelegateIntoNextMiddleware(): void { $shortCode = 'abc123'; - $this->urlShortener->shortCodeToUrl($shortCode, '')->willThrow(ShortUrlNotFoundException::class) - ->shouldBeCalledOnce(); + $this->urlResolver->shortCodeToEnabledShortUrl($shortCode, '')->willThrow(ShortUrlNotFoundException::class) + ->shouldBeCalledOnce(); $delegate = $this->prophesize(RequestHandlerInterface::class); $process = $delegate->handle(Argument::any())->willReturn(new Response()); @@ -50,8 +50,8 @@ class QrCodeActionTest extends TestCase public function anInvalidShortCodeWillReturnNotFoundResponse(): void { $shortCode = 'abc123'; - $this->urlShortener->shortCodeToUrl($shortCode, '')->willThrow(ShortUrlNotFoundException::class) - ->shouldBeCalledOnce(); + $this->urlResolver->shortCodeToEnabledShortUrl($shortCode, '')->willThrow(ShortUrlNotFoundException::class) + ->shouldBeCalledOnce(); $delegate = $this->prophesize(RequestHandlerInterface::class); $process = $delegate->handle(Argument::any())->willReturn(new Response()); @@ -64,8 +64,8 @@ class QrCodeActionTest extends TestCase public function aCorrectRequestReturnsTheQrCodeResponse(): void { $shortCode = 'abc123'; - $this->urlShortener->shortCodeToUrl($shortCode, '')->willReturn(new ShortUrl('')) - ->shouldBeCalledOnce(); + $this->urlResolver->shortCodeToEnabledShortUrl($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 b7060a8e..25e67f4b 100644 --- a/module/Core/test/Action/RedirectActionTest.php +++ b/module/Core/test/Action/RedirectActionTest.php @@ -14,24 +14,24 @@ use Shlinkio\Shlink\Core\Action\RedirectAction; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; use Shlinkio\Shlink\Core\Options; -use Shlinkio\Shlink\Core\Service\UrlShortener; -use Shlinkio\Shlink\Core\Service\VisitsTracker; +use Shlinkio\Shlink\Core\Service\ShortUrl\ShortUrlResolverInterface; +use Shlinkio\Shlink\Core\Service\VisitsTrackerInterface; use function array_key_exists; class RedirectActionTest extends TestCase { private RedirectAction $action; - private ObjectProphecy $urlShortener; + private ObjectProphecy $urlResolver; private ObjectProphecy $visitTracker; public function setUp(): void { - $this->urlShortener = $this->prophesize(UrlShortener::class); - $this->visitTracker = $this->prophesize(VisitsTracker::class); + $this->urlResolver = $this->prophesize(ShortUrlResolverInterface::class); + $this->visitTracker = $this->prophesize(VisitsTrackerInterface::class); $this->action = new RedirectAction( - $this->urlShortener->reveal(), + $this->urlResolver->reveal(), $this->visitTracker->reveal(), new Options\AppOptions(['disableTrackParam' => 'foobar']), ); @@ -45,7 +45,7 @@ class RedirectActionTest extends TestCase { $shortCode = 'abc123'; $shortUrl = new ShortUrl('http://domain.com/foo/bar?some=thing'); - $shortCodeToUrl = $this->urlShortener->shortCodeToUrl($shortCode, '')->willReturn($shortUrl); + $shortCodeToUrl = $this->urlResolver->shortCodeToEnabledShortUrl($shortCode, '')->willReturn($shortUrl); $track = $this->visitTracker->track(Argument::cetera())->will(function (): void { }); @@ -74,8 +74,8 @@ class RedirectActionTest extends TestCase public function nextMiddlewareIsInvokedIfLongUrlIsNotFound(): void { $shortCode = 'abc123'; - $this->urlShortener->shortCodeToUrl($shortCode, '')->willThrow(ShortUrlNotFoundException::class) - ->shouldBeCalledOnce(); + $this->urlResolver->shortCodeToEnabledShortUrl($shortCode, '')->willThrow(ShortUrlNotFoundException::class) + ->shouldBeCalledOnce(); $this->visitTracker->track(Argument::cetera())->shouldNotBeCalled(); $handler = $this->prophesize(RequestHandlerInterface::class); diff --git a/module/Rest/config/dependencies.config.php b/module/Rest/config/dependencies.config.php index 832bf361..6938b6ba 100644 --- a/module/Rest/config/dependencies.config.php +++ b/module/Rest/config/dependencies.config.php @@ -57,7 +57,10 @@ return [ ], Action\ShortUrl\EditShortUrlAction::class => [Service\ShortUrlService::class, 'Logger_Shlink'], Action\ShortUrl\DeleteShortUrlAction::class => [Service\ShortUrl\DeleteShortUrlService::class, 'Logger_Shlink'], - Action\ShortUrl\ResolveShortUrlAction::class => [Service\UrlShortener::class, 'config.url_shortener.domain'], + Action\ShortUrl\ResolveShortUrlAction::class => [ + Service\ShortUrl\ShortUrlResolver::class, + 'config.url_shortener.domain', + ], Action\Visit\GetVisitsAction::class => [Service\VisitsTracker::class, 'Logger_Shlink'], Action\ShortUrl\ListShortUrlsAction::class => [ Service\ShortUrlService::class, diff --git a/module/Rest/src/Action/ShortUrl/ResolveShortUrlAction.php b/module/Rest/src/Action/ShortUrl/ResolveShortUrlAction.php index 9260f597..03458a2c 100644 --- a/module/Rest/src/Action/ShortUrl/ResolveShortUrlAction.php +++ b/module/Rest/src/Action/ShortUrl/ResolveShortUrlAction.php @@ -4,12 +4,11 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Rest\Action\ShortUrl; -use InvalidArgumentException; use Laminas\Diactoros\Response\JsonResponse; use Psr\Http\Message\ResponseInterface as Response; use Psr\Http\Message\ServerRequestInterface as Request; use Psr\Log\LoggerInterface; -use Shlinkio\Shlink\Core\Service\UrlShortenerInterface; +use Shlinkio\Shlink\Core\Service\ShortUrl\ShortUrlResolverInterface; use Shlinkio\Shlink\Core\Transformer\ShortUrlDataTransformer; use Shlinkio\Shlink\Rest\Action\AbstractRestAction; @@ -18,29 +17,26 @@ class ResolveShortUrlAction extends AbstractRestAction protected const ROUTE_PATH = '/short-urls/{shortCode}'; protected const ROUTE_ALLOWED_METHODS = [self::METHOD_GET]; - private UrlShortenerInterface $urlShortener; + private ShortUrlResolverInterface $urlResolver; private array $domainConfig; public function __construct( - UrlShortenerInterface $urlShortener, + ShortUrlResolverInterface $urlResolver, array $domainConfig, ?LoggerInterface $logger = null ) { parent::__construct($logger); - $this->urlShortener = $urlShortener; + $this->urlResolver = $urlResolver; $this->domainConfig = $domainConfig; } - /** - * @throws InvalidArgumentException - */ public function handle(Request $request): Response { $shortCode = $request->getAttribute('shortCode'); $domain = $request->getQueryParams()['domain'] ?? null; $transformer = new ShortUrlDataTransformer($this->domainConfig); - $url = $this->urlShortener->shortCodeToUrl($shortCode, $domain); + $url = $this->urlResolver->shortCodeToShortUrl($shortCode, $domain); return new JsonResponse($transformer->transform($url)); } } diff --git a/module/Rest/test-api/Action/EditShortUrlActionTest.php b/module/Rest/test-api/Action/EditShortUrlActionTest.php index 62b297d9..482accfe 100644 --- a/module/Rest/test-api/Action/EditShortUrlActionTest.php +++ b/module/Rest/test-api/Action/EditShortUrlActionTest.php @@ -18,7 +18,7 @@ class EditShortUrlActionTest extends ApiTestCase /** * @test - * @dataProvider provideDisablingMeta + * @dataProvider provideMeta */ public function metadataCanBeReset(array $meta): void { @@ -30,11 +30,9 @@ class EditShortUrlActionTest extends ApiTestCase 'maxVisits' => null, ]; - // Setting meta that disables the URL should not let it be visited $editWithProvidedMeta = $this->callApiWithKey(self::METHOD_PATCH, $url, [RequestOptions::JSON => $meta]); $metaAfterEditing = $this->findShortUrlMetaByShortCode($shortCode); - // Resetting all meta should allow the URL to be visitable again $editWithResetMeta = $this->callApiWithKey(self::METHOD_PATCH, $url, [ RequestOptions::JSON => $resetMeta, ]); @@ -46,7 +44,7 @@ class EditShortUrlActionTest extends ApiTestCase self::assertArraySubset($meta, $metaAfterEditing); } - public function provideDisablingMeta(): iterable + public function provideMeta(): iterable { $now = Chronos::now(); diff --git a/module/Rest/test-api/Action/ResolveShortUrlActionTest.php b/module/Rest/test-api/Action/ResolveShortUrlActionTest.php index 09f48113..27d9dd69 100644 --- a/module/Rest/test-api/Action/ResolveShortUrlActionTest.php +++ b/module/Rest/test-api/Action/ResolveShortUrlActionTest.php @@ -4,10 +4,42 @@ declare(strict_types=1); namespace ShlinkioApiTest\Shlink\Rest\Action; +use Cake\Chronos\Chronos; +use GuzzleHttp\RequestOptions; use Shlinkio\Shlink\TestUtils\ApiTest\ApiTestCase; +use function sprintf; + class ResolveShortUrlActionTest extends ApiTestCase { + /** + * @test + * @dataProvider provideDisabledMeta + */ + public function shortUrlIsProperlyResolvedEvenWhenNotEnabled(array $disabledMeta): void + { + $shortCode = 'abc123'; + $url = sprintf('/short-urls/%s', $shortCode); + $this->callShortUrl($shortCode); + + $editResp = $this->callApiWithKey(self::METHOD_PATCH, $url, [RequestOptions::JSON => $disabledMeta]); + $visitResp = $this->callShortUrl($shortCode); + $fetchResp = $this->callApiWithKey(self::METHOD_GET, $url); + + $this->assertEquals(self::STATUS_NO_CONTENT, $editResp->getStatusCode()); + $this->assertEquals(self::STATUS_NOT_FOUND, $visitResp->getStatusCode()); + $this->assertEquals(self::STATUS_OK, $fetchResp->getStatusCode()); + } + + public function provideDisabledMeta(): iterable + { + $now = Chronos::now(); + + yield 'future validSince' => [['validSince' => $now->addMonth()->toAtomString()]]; + yield 'past validUntil' => [['validUntil' => $now->subMonth()->toAtomString()]]; + yield 'maxVisits reached' => [['maxVisits' => 1]]; + } + /** @test */ public function tryingToResolveInvalidUrlReturnsNotFoundError(): void { diff --git a/module/Rest/test/Action/ShortUrl/ResolveShortUrlActionTest.php b/module/Rest/test/Action/ShortUrl/ResolveShortUrlActionTest.php index e2932513..067b4e0e 100644 --- a/module/Rest/test/Action/ShortUrl/ResolveShortUrlActionTest.php +++ b/module/Rest/test/Action/ShortUrl/ResolveShortUrlActionTest.php @@ -8,7 +8,7 @@ use Laminas\Diactoros\ServerRequest; use PHPUnit\Framework\TestCase; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\Core\Entity\ShortUrl; -use Shlinkio\Shlink\Core\Service\UrlShortener; +use Shlinkio\Shlink\Core\Service\ShortUrl\ShortUrlResolverInterface; use Shlinkio\Shlink\Rest\Action\ShortUrl\ResolveShortUrlAction; use function strpos; @@ -16,19 +16,19 @@ use function strpos; class ResolveShortUrlActionTest extends TestCase { private ResolveShortUrlAction $action; - private ObjectProphecy $urlShortener; + private ObjectProphecy $urlResolver; public function setUp(): void { - $this->urlShortener = $this->prophesize(UrlShortener::class); - $this->action = new ResolveShortUrlAction($this->urlShortener->reveal(), []); + $this->urlResolver = $this->prophesize(ShortUrlResolverInterface::class); + $this->action = new ResolveShortUrlAction($this->urlResolver->reveal(), []); } /** @test */ public function correctShortCodeReturnsSuccess(): void { $shortCode = 'abc123'; - $this->urlShortener->shortCodeToUrl($shortCode, null)->willReturn( + $this->urlResolver->shortCodeToShortUrl($shortCode, null)->willReturn( new ShortUrl('http://domain.com/foo/bar'), )->shouldBeCalledOnce();