diff --git a/CHANGELOG.md b/CHANGELOG.md index e9a1c86a..74ee51ab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,32 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com), and this project adheres to [Semantic Versioning](https://semver.org). +## 2.0.3 - 2020-01-27 + +#### Added + +* *Nothing* + +#### Changed + +* *Nothing* + +#### Deprecated + +* *Nothing* + +#### Removed + +* *Nothing* + +#### Fixed + +* [#624](https://github.com/shlinkio/shlink/issues/624) Fixed order in which headers for remote IP detection are inspected. +* [#623](https://github.com/shlinkio/shlink/issues/623) Fixed short URLs metadata being impossible to reset. +* [#628](https://github.com/shlinkio/shlink/issues/628) Fixed `GET /short-urls/{shortCode}` REST endpoint returning a 404 for short URLs which are not enabled. +* [#621](https://github.com/shlinkio/shlink/issues/621) Fixed permission denied error when updating same GeoLite file version more than once. + + ## 2.0.2 - 2020-01-12 #### Added diff --git a/composer.json b/composer.json index fb786eec..19b3f557 100644 --- a/composer.json +++ b/composer.json @@ -49,8 +49,8 @@ "pugx/shortid-php": "^0.5", "shlinkio/shlink-common": "^2.5", "shlinkio/shlink-event-dispatcher": "^1.3", - "shlinkio/shlink-installer": "^4.0", - "shlinkio/shlink-ip-geolocation": "^1.3", + "shlinkio/shlink-installer": "^4.0.1", + "shlinkio/shlink-ip-geolocation": "^1.3.1", "symfony/console": "^5.0", "symfony/filesystem": "^5.0", "symfony/lock": "^5.0", @@ -58,6 +58,7 @@ }, "require-dev": { "devster/ubench": "^2.0", + "dms/phpunit-arraysubset-asserts": "^0.1.0", "eaglewu/swoole-ide-helper": "dev-master", "infection/infection": "^0.15.0", "phpstan/phpstan": "^0.12.3", diff --git a/config/autoload/client-detection.global.php b/config/autoload/client-detection.global.php index 3c6fb8b2..a49b3d93 100644 --- a/config/autoload/client-detection.global.php +++ b/config/autoload/client-detection.global.php @@ -7,11 +7,11 @@ return [ 'ip_address_resolution' => [ 'headers_to_inspect' => [ 'CF-Connecting-IP', - 'True-Client-IP', - 'X-Real-IP', - 'Forwarded', 'X-Forwarded-For', 'X-Forwarded', + 'Forwarded', + 'True-Client-IP', + 'X-Real-IP', 'X-Cluster-Client-Ip', 'Client-Ip', ], diff --git a/config/autoload/geolite2.global.php b/config/autoload/geolite2.global.php index 97e8c381..10f58042 100644 --- a/config/autoload/geolite2.global.php +++ b/config/autoload/geolite2.global.php @@ -7,9 +7,7 @@ return [ 'geolite2' => [ 'db_location' => __DIR__ . '/../../data/GeoLite2-City.mmdb', 'temp_dir' => sys_get_temp_dir(), - 'download_from' => - 'https://download.maxmind.com/app/geoip_download' - . '?edition_id=GeoLite2-City&license_key=G4Lm0C60yJsnkdPi&suffix=tar.gz', + 'license_key' => 'G4Lm0C60yJsnkdPi', ], ]; diff --git a/docker/README.md b/docker/README.md index 4b1fdea1..0af66442 100644 --- a/docker/README.md +++ b/docker/README.md @@ -1,6 +1,6 @@ # Shlink Docker image -[![Docker build status](https://img.shields.io/docker/cloud/build/shlinkio/shlink.svg?style=flat-square)](https://hub.docker.com/r/shlinkio/shlink/) +[![Docker build status](https://img.shields.io/docker/build/shlinkio/shlink.svg?style=flat-square)](https://hub.docker.com/r/shlinkio/shlink/) [![Docker pulls](https://img.shields.io/docker/pulls/shlinkio/shlink.svg?style=flat-square)](https://hub.docker.com/r/shlinkio/shlink/) This image provides an easy way to set up [shlink](https://shlink.io) on a container-based runtime. 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/GenerateShortUrlCommand.php b/module/CLI/src/Command/ShortUrl/GenerateShortUrlCommand.php index 1e9b41ce..28d192b1 100644 --- a/module/CLI/src/Command/ShortUrl/GenerateShortUrlCommand.php +++ b/module/CLI/src/Command/ShortUrl/GenerateShortUrlCommand.php @@ -10,6 +10,7 @@ use Shlinkio\Shlink\Core\Exception\InvalidUrlException; use Shlinkio\Shlink\Core\Exception\NonUniqueSlugException; use Shlinkio\Shlink\Core\Model\ShortUrlMeta; use Shlinkio\Shlink\Core\Service\UrlShortenerInterface; +use Shlinkio\Shlink\Core\Validation\ShortUrlMetaInputFilter; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; @@ -121,14 +122,14 @@ class GenerateShortUrlCommand extends Command $shortUrl = $this->urlShortener->urlToShortCode( new Uri($longUrl), $tags, - ShortUrlMeta::createFromParams( - $input->getOption('validSince'), - $input->getOption('validUntil'), - $customSlug, - $maxVisits !== null ? (int) $maxVisits : null, - $input->getOption('findIfExists'), - $input->getOption('domain'), - ), + ShortUrlMeta::fromRawData([ + ShortUrlMetaInputFilter::VALID_SINCE => $input->getOption('validSince'), + ShortUrlMetaInputFilter::VALID_UNTIL => $input->getOption('validUntil'), + ShortUrlMetaInputFilter::CUSTOM_SLUG => $customSlug, + ShortUrlMetaInputFilter::MAX_VISITS => $maxVisits !== null ? (int) $maxVisits : null, + ShortUrlMetaInputFilter::FIND_IF_EXISTS => $input->getOption('findIfExists'), + ShortUrlMetaInputFilter::DOMAIN => $input->getOption('domain'), + ]), ); $io->writeln([ 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..e260896d 100644 --- a/module/Core/src/Entity/ShortUrl.php +++ b/module/Core/src/Entity/ShortUrl.php @@ -135,7 +135,6 @@ class ShortUrl extends AbstractEntity /** * @param Collection|Visit[] $visits - * @return ShortUrl * @internal */ public function setVisits(Collection $visits): self @@ -149,9 +148,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 +201,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/Model/ShortUrlMeta.php b/module/Core/src/Model/ShortUrlMeta.php index f9491d02..f0c487b7 100644 --- a/module/Core/src/Model/ShortUrlMeta.php +++ b/module/Core/src/Model/ShortUrlMeta.php @@ -9,11 +9,16 @@ use DateTimeInterface; use Shlinkio\Shlink\Core\Exception\ValidationException; use Shlinkio\Shlink\Core\Validation\ShortUrlMetaInputFilter; +use function array_key_exists; + final class ShortUrlMeta { + private bool $validSincePropWasProvided = false; private ?Chronos $validSince = null; + private bool $validUntilPropWasProvided = false; private ?Chronos $validUntil = null; private ?string $customSlug = null; + private bool $maxVisitsPropWasProvided = false; private ?int $maxVisits = null; private ?bool $findIfExists = null; private ?string $domain = null; @@ -32,44 +37,13 @@ final class ShortUrlMeta * @param array $data * @throws ValidationException */ - public static function createFromRawData(array $data): self + public static function fromRawData(array $data): self { $instance = new self(); $instance->validate($data); return $instance; } - /** - * @param string|Chronos|null $validSince - * @param string|Chronos|null $validUntil - * @param string|null $customSlug - * @param int|null $maxVisits - * @param bool|null $findIfExists - * @param string|null $domain - * @throws ValidationException - */ - public static function createFromParams( // phpcs:ignore - $validSince = null, - $validUntil = null, - $customSlug = null, - $maxVisits = null, - $findIfExists = null, - $domain = null - ): self { - // We do not type hint the arguments because that will be done by the validation process and we would get a - // type error if any of them do not match - $instance = new self(); - $instance->validate([ - ShortUrlMetaInputFilter::VALID_SINCE => $validSince, - ShortUrlMetaInputFilter::VALID_UNTIL => $validUntil, - ShortUrlMetaInputFilter::CUSTOM_SLUG => $customSlug, - ShortUrlMetaInputFilter::MAX_VISITS => $maxVisits, - ShortUrlMetaInputFilter::FIND_IF_EXISTS => $findIfExists, - ShortUrlMetaInputFilter::DOMAIN => $domain, - ]); - return $instance; - } - /** * @param array $data * @throws ValidationException @@ -82,10 +56,13 @@ final class ShortUrlMeta } $this->validSince = $this->parseDateField($inputFilter->getValue(ShortUrlMetaInputFilter::VALID_SINCE)); + $this->validSincePropWasProvided = array_key_exists(ShortUrlMetaInputFilter::VALID_SINCE, $data); $this->validUntil = $this->parseDateField($inputFilter->getValue(ShortUrlMetaInputFilter::VALID_UNTIL)); + $this->validUntilPropWasProvided = array_key_exists(ShortUrlMetaInputFilter::VALID_UNTIL, $data); $this->customSlug = $inputFilter->getValue(ShortUrlMetaInputFilter::CUSTOM_SLUG); $maxVisits = $inputFilter->getValue(ShortUrlMetaInputFilter::MAX_VISITS); $this->maxVisits = $maxVisits !== null ? (int) $maxVisits : null; + $this->maxVisitsPropWasProvided = array_key_exists(ShortUrlMetaInputFilter::MAX_VISITS, $data); $this->findIfExists = $inputFilter->getValue(ShortUrlMetaInputFilter::FIND_IF_EXISTS); $this->domain = $inputFilter->getValue(ShortUrlMetaInputFilter::DOMAIN); } @@ -113,7 +90,7 @@ final class ShortUrlMeta public function hasValidSince(): bool { - return $this->validSince !== null; + return $this->validSincePropWasProvided; } public function getValidUntil(): ?Chronos @@ -123,7 +100,7 @@ final class ShortUrlMeta public function hasValidUntil(): bool { - return $this->validUntil !== null; + return $this->validUntilPropWasProvided; } public function getCustomSlug(): ?string @@ -143,7 +120,7 @@ final class ShortUrlMeta public function hasMaxVisits(): bool { - return $this->maxVisits !== null; + return $this->maxVisitsPropWasProvided; } public function findIfExists(): bool diff --git a/module/Core/src/Repository/ShortUrlRepository.php b/module/Core/src/Repository/ShortUrlRepository.php index dafc841f..a334a838 100644 --- a/module/Core/src/Repository/ShortUrlRepository.php +++ b/module/Core/src/Repository/ShortUrlRepository.php @@ -4,7 +4,6 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Repository; -use Cake\Chronos\Chronos; use Doctrine\ORM\EntityRepository; use Doctrine\ORM\QueryBuilder; use Shlinkio\Shlink\Common\Util\DateRange; @@ -146,8 +145,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 +153,6 @@ DQL; $query->setMaxResults(1) ->setParameters([ 'shortCode' => $shortCode, - 'now' => Chronos::now(), 'domain' => $domain, ]); @@ -166,9 +162,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 @@ +verifyShortCodeUniqueness($meta, $shortUrlToBeCreated); } } - - /** - * @throws ShortUrlNotFoundException - * @fixme Move this method to a different service - */ - public function shortCodeToUrl(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; - } } diff --git a/module/Core/src/Service/UrlShortenerInterface.php b/module/Core/src/Service/UrlShortenerInterface.php index ee9cc343..802eb048 100644 --- a/module/Core/src/Service/UrlShortenerInterface.php +++ b/module/Core/src/Service/UrlShortenerInterface.php @@ -8,7 +8,6 @@ use Psr\Http\Message\UriInterface; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Exception\InvalidUrlException; use Shlinkio\Shlink\Core\Exception\NonUniqueSlugException; -use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; use Shlinkio\Shlink\Core\Model\ShortUrlMeta; interface UrlShortenerInterface @@ -19,9 +18,4 @@ interface UrlShortenerInterface * @throws InvalidUrlException */ public function urlToShortCode(UriInterface $url, array $tags, ShortUrlMeta $meta): ShortUrl; - - /** - * @throws ShortUrlNotFoundException - */ - public function shortCodeToUrl(string $shortCode, ?string $domain = null): ShortUrl; } diff --git a/module/Core/test-db/Repository/ShortUrlRepositoryTest.php b/module/Core/test-db/Repository/ShortUrlRepositoryTest.php index 81f32bd8..f742c6eb 100644 --- a/module/Core/test-db/Repository/ShortUrlRepositoryTest.php +++ b/module/Core/test-db/Repository/ShortUrlRepositoryTest.php @@ -38,41 +38,17 @@ class ShortUrlRepositoryTest extends DatabaseTestCase /** @test */ public function findOneByShortCodeReturnsProperData(): void { - $regularOne = new ShortUrl('foo', ShortUrlMeta::createFromParams(null, null, 'foo')); + $regularOne = new ShortUrl('foo', ShortUrlMeta::fromRawData(['customSlug' => 'foo'])); $this->getEntityManager()->persist($regularOne); - $notYetValid = new ShortUrl( - 'bar', - ShortUrlMeta::createFromParams(Chronos::now()->addMonth(), null, 'bar_very_long_text'), - ); - $this->getEntityManager()->persist($notYetValid); - - $expired = new ShortUrl('expired', ShortUrlMeta::createFromParams(null, Chronos::now()->subMonth(), 'expired')); - $this->getEntityManager()->persist($expired); - - $allVisitsComplete = new ShortUrl('baz', ShortUrlMeta::createFromRawData([ - 'maxVisits' => 3, - 'customSlug' => 'baz', - ])); - $visits = []; - for ($i = 0; $i < 3; $i++) { - $visit = new Visit($allVisitsComplete, Visitor::emptyInstance()); - $this->getEntityManager()->persist($visit); - $visits[] = $visit; - } - $allVisitsComplete->setVisits(new ArrayCollection($visits)); - $this->getEntityManager()->persist($allVisitsComplete); - - $withDomain = new ShortUrl('foo', ShortUrlMeta::createFromRawData([ - 'domain' => 'example.com', - 'customSlug' => 'domain-short-code', - ])); + $withDomain = new ShortUrl('foo', ShortUrlMeta::fromRawData( + ['domain' => 'example.com', 'customSlug' => 'domain-short-code'], + )); $this->getEntityManager()->persist($withDomain); - $withDomainDuplicatingRegular = new ShortUrl('foo_with_domain', ShortUrlMeta::createFromRawData([ - 'domain' => 'doma.in', - 'customSlug' => 'foo', - ])); + $withDomainDuplicatingRegular = new ShortUrl('foo_with_domain', ShortUrlMeta::fromRawData( + ['domain' => 'doma.in', 'customSlug' => 'foo'], + )); $this->getEntityManager()->persist($withDomainDuplicatingRegular); $this->getEntityManager()->flush(); @@ -91,9 +67,6 @@ class ShortUrlRepositoryTest extends DatabaseTestCase $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->findOneByShortCode($notYetValid->getShortCode())); - $this->assertNull($this->repo->findOneByShortCode($expired->getShortCode())); - $this->assertNull($this->repo->findOneByShortCode($allVisitsComplete->getShortCode())); } /** @test */ @@ -187,12 +160,12 @@ class ShortUrlRepositoryTest extends DatabaseTestCase /** @test */ public function shortCodeIsInUseLooksForShortUrlInProperSetOfTables(): void { - $shortUrlWithoutDomain = new ShortUrl('foo', ShortUrlMeta::createFromRawData(['customSlug' => 'my-cool-slug'])); + $shortUrlWithoutDomain = new ShortUrl('foo', ShortUrlMeta::fromRawData(['customSlug' => 'my-cool-slug'])); $this->getEntityManager()->persist($shortUrlWithoutDomain); $shortUrlWithDomain = new ShortUrl( 'foo', - ShortUrlMeta::createFromRawData(['domain' => 'doma.in', 'customSlug' => 'another-slug']), + ShortUrlMeta::fromRawData(['domain' => 'doma.in', 'customSlug' => 'another-slug']), ); $this->getEntityManager()->persist($shortUrlWithDomain); diff --git a/module/Core/test/Action/PixelActionTest.php b/module/Core/test/Action/PixelActionTest.php index fc9b85a1..ed9d0774 100644 --- a/module/Core/test/Action/PixelActionTest.php +++ b/module/Core/test/Action/PixelActionTest.php @@ -13,22 +13,22 @@ use Shlinkio\Shlink\Common\Response\PixelResponse; use Shlinkio\Shlink\Core\Action\PixelAction; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Options\AppOptions; -use Shlinkio\Shlink\Core\Service\UrlShortener; +use Shlinkio\Shlink\Core\Service\ShortUrl\ShortUrlResolverInterface; use Shlinkio\Shlink\Core\Service\VisitsTracker; class PixelActionTest extends TestCase { private PixelAction $action; - private ObjectProphecy $urlShortener; + private ObjectProphecy $urlResolver; private ObjectProphecy $visitTracker; public function setUp(): void { - $this->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/Core/test/Entity/ShortUrlTest.php b/module/Core/test/Entity/ShortUrlTest.php index 98404ca1..9aba83fa 100644 --- a/module/Core/test/Entity/ShortUrlTest.php +++ b/module/Core/test/Entity/ShortUrlTest.php @@ -28,7 +28,7 @@ class ShortUrlTest extends TestCase public function provideInvalidShortUrls(): iterable { yield 'with custom slug' => [ - new ShortUrl('', ShortUrlMeta::createFromRawData(['customSlug' => 'custom-slug'])), + new ShortUrl('', ShortUrlMeta::fromRawData(['customSlug' => 'custom-slug'])), 'The short code cannot be regenerated on ShortUrls where a custom slug was provided.', ]; yield 'already persisted' => [ diff --git a/module/Core/test/Model/ShortUrlMetaTest.php b/module/Core/test/Model/ShortUrlMetaTest.php index fb02fcbc..13c5ae14 100644 --- a/module/Core/test/Model/ShortUrlMetaTest.php +++ b/module/Core/test/Model/ShortUrlMetaTest.php @@ -21,7 +21,7 @@ class ShortUrlMetaTest extends TestCase public function exceptionIsThrownIfProvidedDataIsInvalid(array $data): void { $this->expectException(ValidationException::class); - ShortUrlMeta::createFromRawData($data); + ShortUrlMeta::fromRawData($data); } public function provideInvalidData(): iterable @@ -49,7 +49,9 @@ class ShortUrlMetaTest extends TestCase /** @test */ public function properlyCreatedInstanceReturnsValues(): void { - $meta = ShortUrlMeta::createFromParams(Chronos::parse('2015-01-01')->toAtomString(), null, 'foobar'); + $meta = ShortUrlMeta::fromRawData( + ['validSince' => Chronos::parse('2015-01-01')->toAtomString(), 'customSlug' => 'foobar'], + ); $this->assertTrue($meta->hasValidSince()); $this->assertEquals(Chronos::parse('2015-01-01'), $meta->getValidSince()); diff --git a/module/Core/test/Service/ShortUrl/ShortUrlResolverTest.php b/module/Core/test/Service/ShortUrl/ShortUrlResolverTest.php new file mode 100644 index 00000000..58f79606 --- /dev/null +++ b/module/Core/test/Service/ShortUrl/ShortUrlResolverTest.php @@ -0,0 +1,135 @@ +em = $this->prophesize(EntityManagerInterface::class); + $this->urlResolver = new ShortUrlResolver($this->em->reveal()); + } + + /** @test */ + public function shortCodeIsProperlyParsed(): void + { + $shortUrl = new ShortUrl('expected_url'); + $shortCode = $shortUrl->getShortCode(); + + $repo = $this->prophesize(ShortUrlRepositoryInterface::class); + $findOneByShortCode = $repo->findOneByShortCode($shortCode, null)->willReturn($shortUrl); + $getRepo = $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal()); + + $result = $this->urlResolver->shortCodeToShortUrl($shortCode); + + $this->assertSame($shortUrl, $result); + $findOneByShortCode->shouldHaveBeenCalledOnce(); + $getRepo->shouldHaveBeenCalledOnce(); + } + + /** @test */ + public function exceptionIsThrownIfShortcodeIsNotFound(): void + { + $shortCode = 'abc123'; + + $repo = $this->prophesize(ShortUrlRepositoryInterface::class); + $findOneByShortCode = $repo->findOneByShortCode($shortCode, null)->willReturn(null); + $getRepo = $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal()); + + $this->expectException(ShortUrlNotFoundException::class); + $findOneByShortCode->shouldBeCalledOnce(); + $getRepo->shouldBeCalledOnce(); + + $this->urlResolver->shortCodeToShortUrl($shortCode); + } + + /** @test */ + public function shortCodeToEnabledShortUrlProperlyParsesShortCode(): void + { + $shortUrl = new ShortUrl('expected_url'); + $shortCode = $shortUrl->getShortCode(); + + $repo = $this->prophesize(ShortUrlRepositoryInterface::class); + $findOneByShortCode = $repo->findOneByShortCode($shortCode, null)->willReturn($shortUrl); + $getRepo = $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal()); + + $result = $this->urlResolver->shortCodeToEnabledShortUrl($shortCode); + + $this->assertSame($shortUrl, $result); + $findOneByShortCode->shouldHaveBeenCalledOnce(); + $getRepo->shouldHaveBeenCalledOnce(); + } + + /** + * @test + * @dataProvider provideDisabledShortUrls + */ + public function shortCodeToEnabledShortUrlThrowsExceptionIfUrlIsNotEnabled(ShortUrl $shortUrl): void + { + $shortCode = $shortUrl->getShortCode(); + + $repo = $this->prophesize(ShortUrlRepositoryInterface::class); + $findOneByShortCode = $repo->findOneByShortCode($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); + } + + public function provideDisabledShortUrls(): iterable + { + $now = Chronos::now(); + + yield 'maxVisits reached' => [(function () { + $shortUrl = new ShortUrl('', ShortUrlMeta::fromRawData(['maxVisits' => 3])); + $shortUrl->setVisits(new ArrayCollection(map( + range(0, 4), + fn () => new Visit($shortUrl, Visitor::emptyInstance()), + ))); + + return $shortUrl; + })()]; + yield 'future validSince' => [new ShortUrl('', ShortUrlMeta::fromRawData([ + 'validSince' => $now->addMonth()->toAtomString(), + ]))]; + yield 'past validUntil' => [new ShortUrl('', ShortUrlMeta::fromRawData([ + 'validUntil' => $now->subMonth()->toAtomString(), + ]))]; + yield 'mixed' => [(function () use ($now) { + $shortUrl = new ShortUrl('', ShortUrlMeta::fromRawData([ + 'maxVisits' => 3, + 'validUntil' => $now->subMonth()->toAtomString(), + ])); + $shortUrl->setVisits(new ArrayCollection(map( + range(0, 4), + fn () => new Visit($shortUrl, Visitor::emptyInstance()), + ))); + + return $shortUrl; + })()]; + } +} diff --git a/module/Core/test/Service/ShortUrlServiceTest.php b/module/Core/test/Service/ShortUrlServiceTest.php index 93cb7ad0..719c69d5 100644 --- a/module/Core/test/Service/ShortUrlServiceTest.php +++ b/module/Core/test/Service/ShortUrlServiceTest.php @@ -93,12 +93,11 @@ class ShortUrlServiceTest extends TestCase $getRepo = $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal()); $flush = $this->em->flush()->willReturn(null); - $result = $this->service->updateMetadataByShortCode('abc123', ShortUrlMeta::createFromParams( - Chronos::parse('2017-01-01 00:00:00')->toAtomString(), - Chronos::parse('2017-01-05 00:00:00')->toAtomString(), - null, - 5, - )); + $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, + ])); $this->assertSame($shortUrl, $result); $this->assertEquals(Chronos::parse('2017-01-01 00:00:00'), $shortUrl->getValidSince()); diff --git a/module/Core/test/Service/UrlShortenerTest.php b/module/Core/test/Service/UrlShortenerTest.php index 2ec3024d..a0392489 100644 --- a/module/Core/test/Service/UrlShortenerTest.php +++ b/module/Core/test/Service/UrlShortenerTest.php @@ -19,7 +19,6 @@ use Shlinkio\Shlink\Core\Exception\NonUniqueSlugException; use Shlinkio\Shlink\Core\Model\ShortUrlMeta; use Shlinkio\Shlink\Core\Options\UrlShortenerOptions; use Shlinkio\Shlink\Core\Repository\ShortUrlRepository; -use Shlinkio\Shlink\Core\Repository\ShortUrlRepositoryInterface; use Shlinkio\Shlink\Core\Service\UrlShortener; use Shlinkio\Shlink\Core\Util\UrlValidatorInterface; @@ -153,7 +152,7 @@ class UrlShortenerTest extends TestCase $this->urlShortener->urlToShortCode( new Uri('http://foobar.com/12345/hello?foo=bar'), [], - ShortUrlMeta::createFromRawData(['customSlug' => 'custom-slug']), + ShortUrlMeta::fromRawData(['customSlug' => 'custom-slug']), ); } @@ -183,49 +182,49 @@ class UrlShortenerTest extends TestCase { $url = 'http://foo.com'; - yield [$url, [], ShortUrlMeta::createFromRawData(['findIfExists' => true]), new ShortUrl($url)]; - yield [$url, [], ShortUrlMeta::createFromRawData( + yield [$url, [], ShortUrlMeta::fromRawData(['findIfExists' => true]), new ShortUrl($url)]; + yield [$url, [], ShortUrlMeta::fromRawData( ['findIfExists' => true, 'customSlug' => 'foo'], ), new ShortUrl($url)]; yield [ $url, ['foo', 'bar'], - ShortUrlMeta::createFromRawData(['findIfExists' => true]), + ShortUrlMeta::fromRawData(['findIfExists' => true]), (new ShortUrl($url))->setTags(new ArrayCollection([new Tag('bar'), new Tag('foo')])), ]; yield [ $url, [], - ShortUrlMeta::createFromRawData(['findIfExists' => true, 'maxVisits' => 3]), - new ShortUrl($url, ShortUrlMeta::createFromRawData(['maxVisits' => 3])), + ShortUrlMeta::fromRawData(['findIfExists' => true, 'maxVisits' => 3]), + new ShortUrl($url, ShortUrlMeta::fromRawData(['maxVisits' => 3])), ]; yield [ $url, [], - ShortUrlMeta::createFromRawData(['findIfExists' => true, 'validSince' => Chronos::parse('2017-01-01')]), - new ShortUrl($url, ShortUrlMeta::createFromRawData(['validSince' => Chronos::parse('2017-01-01')])), + ShortUrlMeta::fromRawData(['findIfExists' => true, 'validSince' => Chronos::parse('2017-01-01')]), + new ShortUrl($url, ShortUrlMeta::fromRawData(['validSince' => Chronos::parse('2017-01-01')])), ]; yield [ $url, [], - ShortUrlMeta::createFromRawData(['findIfExists' => true, 'validUntil' => Chronos::parse('2017-01-01')]), - new ShortUrl($url, ShortUrlMeta::createFromRawData(['validUntil' => Chronos::parse('2017-01-01')])), + ShortUrlMeta::fromRawData(['findIfExists' => true, 'validUntil' => Chronos::parse('2017-01-01')]), + new ShortUrl($url, ShortUrlMeta::fromRawData(['validUntil' => Chronos::parse('2017-01-01')])), ]; yield [ $url, [], - ShortUrlMeta::createFromRawData(['findIfExists' => true, 'domain' => 'example.com']), - new ShortUrl($url, ShortUrlMeta::createFromRawData(['domain' => 'example.com'])), + ShortUrlMeta::fromRawData(['findIfExists' => true, 'domain' => 'example.com']), + new ShortUrl($url, ShortUrlMeta::fromRawData(['domain' => 'example.com'])), ]; yield [ $url, ['baz', 'foo', 'bar'], - ShortUrlMeta::createFromRawData([ + ShortUrlMeta::fromRawData([ 'findIfExists' => true, 'validUntil' => Chronos::parse('2017-01-01'), 'maxVisits' => 4, ]), - (new ShortUrl($url, ShortUrlMeta::createFromRawData([ + (new ShortUrl($url, ShortUrlMeta::fromRawData([ 'validUntil' => Chronos::parse('2017-01-01'), 'maxVisits' => 4, ])))->setTags(new ArrayCollection([new Tag('foo'), new Tag('bar'), new Tag('baz')])), @@ -237,7 +236,7 @@ class UrlShortenerTest extends TestCase { $url = 'http://foo.com'; $tags = ['baz', 'foo', 'bar']; - $meta = ShortUrlMeta::createFromRawData([ + $meta = ShortUrlMeta::fromRawData([ 'findIfExists' => true, 'validUntil' => Chronos::parse('2017-01-01'), 'maxVisits' => 4, @@ -260,18 +259,4 @@ class UrlShortenerTest extends TestCase $findExisting->shouldHaveBeenCalledOnce(); $getRepo->shouldHaveBeenCalledOnce(); } - - /** @test */ - public function shortCodeIsProperlyParsed(): void - { - $shortUrl = new ShortUrl('expected_url'); - $shortCode = $shortUrl->getShortCode(); - - $repo = $this->prophesize(ShortUrlRepositoryInterface::class); - $repo->findOneByShortCode($shortCode, null)->willReturn($shortUrl); - $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal()); - - $url = $this->urlShortener->shortCodeToUrl($shortCode); - $this->assertSame($shortUrl, $url); - } } diff --git a/module/Core/test/Transformer/ShortUrlDataTransformerTest.php b/module/Core/test/Transformer/ShortUrlDataTransformerTest.php index 094e09d6..a65b9506 100644 --- a/module/Core/test/Transformer/ShortUrlDataTransformerTest.php +++ b/module/Core/test/Transformer/ShortUrlDataTransformerTest.php @@ -42,13 +42,13 @@ class ShortUrlDataTransformerTest extends TestCase 'validUntil' => null, 'maxVisits' => null, ]]; - yield 'max visits only' => [new ShortUrl('', ShortUrlMeta::createFromParams(null, null, null, $maxVisits)), [ + yield 'max visits only' => [new ShortUrl('', ShortUrlMeta::fromRawData(['maxVisits' => $maxVisits])), [ 'validSince' => null, 'validUntil' => null, 'maxVisits' => $maxVisits, ]]; yield 'max visits and valid since' => [ - new ShortUrl('', ShortUrlMeta::createFromParams($now, null, null, $maxVisits)), + new ShortUrl('', ShortUrlMeta::fromRawData(['validSince' => $now, 'maxVisits' => $maxVisits])), [ 'validSince' => $now->toAtomString(), 'validUntil' => null, @@ -56,7 +56,9 @@ class ShortUrlDataTransformerTest extends TestCase ], ]; yield 'both dates' => [ - new ShortUrl('', ShortUrlMeta::createFromParams($now, $now->subDays(10))), + new ShortUrl('', ShortUrlMeta::fromRawData( + ['validSince' => $now, 'validUntil' => $now->subDays(10)], + )), [ 'validSince' => $now->toAtomString(), 'validUntil' => $now->subDays(10)->toAtomString(), @@ -64,7 +66,9 @@ class ShortUrlDataTransformerTest extends TestCase ], ]; yield 'everything' => [ - new ShortUrl('', ShortUrlMeta::createFromParams($now, $now->subDays(5), null, $maxVisits)), + new ShortUrl('', ShortUrlMeta::fromRawData( + ['validSince' => $now, 'validUntil' => $now->subDays(5), 'maxVisits' => $maxVisits], + )), [ 'validSince' => $now->toAtomString(), 'validUntil' => $now->subDays(5)->toAtomString(), 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/CreateShortUrlAction.php b/module/Rest/src/Action/ShortUrl/CreateShortUrlAction.php index e85d2465..489d1277 100644 --- a/module/Rest/src/Action/ShortUrl/CreateShortUrlAction.php +++ b/module/Rest/src/Action/ShortUrl/CreateShortUrlAction.php @@ -27,15 +27,7 @@ class CreateShortUrlAction extends AbstractCreateShortUrlAction ]); } - $meta = ShortUrlMeta::createFromParams( - $postData['validSince'] ?? null, - $postData['validUntil'] ?? null, - $postData['customSlug'] ?? null, - $postData['maxVisits'] ?? null, - $postData['findIfExists'] ?? null, - $postData['domain'] ?? null, - ); - + $meta = ShortUrlMeta::fromRawData($postData); return new CreateShortUrlData(new Uri($postData['longUrl']), (array) ($postData['tags'] ?? []), $meta); } } diff --git a/module/Rest/src/Action/ShortUrl/EditShortUrlAction.php b/module/Rest/src/Action/ShortUrl/EditShortUrlAction.php index 4270f326..a6bc5538 100644 --- a/module/Rest/src/Action/ShortUrl/EditShortUrlAction.php +++ b/module/Rest/src/Action/ShortUrl/EditShortUrlAction.php @@ -30,7 +30,7 @@ class EditShortUrlAction extends AbstractRestAction $postData = (array) $request->getParsedBody(); $shortCode = $request->getAttribute('shortCode', ''); - $this->shortUrlService->updateMetadataByShortCode($shortCode, ShortUrlMeta::createFromRawData($postData)); + $this->shortUrlService->updateMetadataByShortCode($shortCode, ShortUrlMeta::fromRawData($postData)); return new EmptyResponse(); } } 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 024ffb79..482accfe 100644 --- a/module/Rest/test-api/Action/EditShortUrlActionTest.php +++ b/module/Rest/test-api/Action/EditShortUrlActionTest.php @@ -4,11 +4,71 @@ declare(strict_types=1); namespace ShlinkioApiTest\Shlink\Rest\Action; +use Cake\Chronos\Chronos; +use DMS\PHPUnitExtensions\ArraySubset\ArraySubsetAsserts; use GuzzleHttp\RequestOptions; use Shlinkio\Shlink\TestUtils\ApiTest\ApiTestCase; +use function Functional\first; +use function sprintf; + class EditShortUrlActionTest extends ApiTestCase { + use ArraySubsetAsserts; + + /** + * @test + * @dataProvider provideMeta + */ + public function metadataCanBeReset(array $meta): void + { + $shortCode = 'abc123'; + $url = sprintf('/short-urls/%s', $shortCode); + $resetMeta = [ + 'validSince' => null, + 'validUntil' => null, + 'maxVisits' => null, + ]; + + $editWithProvidedMeta = $this->callApiWithKey(self::METHOD_PATCH, $url, [RequestOptions::JSON => $meta]); + $metaAfterEditing = $this->findShortUrlMetaByShortCode($shortCode); + + $editWithResetMeta = $this->callApiWithKey(self::METHOD_PATCH, $url, [ + RequestOptions::JSON => $resetMeta, + ]); + $metaAfterResetting = $this->findShortUrlMetaByShortCode($shortCode); + + $this->assertEquals(self::STATUS_NO_CONTENT, $editWithProvidedMeta->getStatusCode()); + $this->assertEquals(self::STATUS_NO_CONTENT, $editWithResetMeta->getStatusCode()); + $this->assertEquals($resetMeta, $metaAfterResetting); + self::assertArraySubset($meta, $metaAfterEditing); + } + + public function provideMeta(): iterable + { + $now = Chronos::now(); + + yield [['validSince' => $now->addMonth()->toAtomString()]]; + yield [['validUntil' => $now->subMonth()->toAtomString()]]; + yield [['maxVisits' => 20]]; + yield [['validUntil' => $now->addYear()->toAtomString(), 'maxVisits' => 100]]; + yield [[ + 'validSince' => $now->subYear()->toAtomString(), + 'validUntil' => $now->addYear()->toAtomString(), + 'maxVisits' => 100, + ]]; + } + + 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); + + return $matchingShortUrl['meta'] ?? null; + } + /** @test */ public function tryingToEditInvalidUrlReturnsNotFoundError(): void { 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-api/Fixtures/ShortUrlsFixture.php b/module/Rest/test-api/Fixtures/ShortUrlsFixture.php index 0b5a841a..d7566063 100644 --- a/module/Rest/test-api/Fixtures/ShortUrlsFixture.php +++ b/module/Rest/test-api/Fixtures/ShortUrlsFixture.php @@ -20,32 +20,32 @@ class ShortUrlsFixture extends AbstractFixture public function load(ObjectManager $manager): void { $abcShortUrl = $this->setShortUrlDate( - new ShortUrl('https://shlink.io', ShortUrlMeta::createFromRawData(['customSlug' => 'abc123'])), + new ShortUrl('https://shlink.io', ShortUrlMeta::fromRawData(['customSlug' => 'abc123'])), '2018-05-01', ); $manager->persist($abcShortUrl); $defShortUrl = $this->setShortUrlDate(new ShortUrl( 'https://blog.alejandrocelaya.com/2017/12/09/acmailer-7-0-the-most-important-release-in-a-long-time/', - ShortUrlMeta::createFromParams(Chronos::parse('2020-05-01'), null, 'def456'), + ShortUrlMeta::fromRawData(['validSince' => Chronos::parse('2020-05-01'), 'customSlug' => 'def456']), ), '2019-01-01 00:00:10'); $manager->persist($defShortUrl); $customShortUrl = $this->setShortUrlDate(new ShortUrl( 'https://shlink.io', - ShortUrlMeta::createFromParams(null, null, 'custom', 2), + ShortUrlMeta::fromRawData(['customSlug' => 'custom', 'maxVisits' => 2]), ), '2019-01-01 00:00:20'); $manager->persist($customShortUrl); $withDomainShortUrl = $this->setShortUrlDate(new ShortUrl( 'https://blog.alejandrocelaya.com/2019/04/27/considerations-to-properly-use-open-source-software-projects/', - ShortUrlMeta::createFromRawData(['domain' => 'example.com', 'customSlug' => 'ghi789']), + ShortUrlMeta::fromRawData(['domain' => 'example.com', 'customSlug' => 'ghi789']), ), '2019-01-01 00:00:30'); $manager->persist($withDomainShortUrl); $withDomainAndSlugShortUrl = $this->setShortUrlDate(new ShortUrl( 'https://google.com', - ShortUrlMeta::createFromRawData(['domain' => 'some-domain.com', 'customSlug' => 'custom-with-domain']), + ShortUrlMeta::fromRawData(['domain' => 'some-domain.com', 'customSlug' => 'custom-with-domain']), ), '2018-10-20'); $manager->persist($withDomainAndSlugShortUrl); diff --git a/module/Rest/test/Action/ShortUrl/CreateShortUrlActionTest.php b/module/Rest/test/Action/ShortUrl/CreateShortUrlActionTest.php index b459765c..3a343d60 100644 --- a/module/Rest/test/Action/ShortUrl/CreateShortUrlActionTest.php +++ b/module/Rest/test/Action/ShortUrl/CreateShortUrlActionTest.php @@ -76,7 +76,7 @@ class CreateShortUrlActionTest extends TestCase ]; yield [['longUrl' => 'http://www.domain.com/foo/bar'], ShortUrlMeta::createEmpty()]; - yield [$fullMeta, ShortUrlMeta::createFromRawData($fullMeta)]; + yield [$fullMeta, ShortUrlMeta::fromRawData($fullMeta)]; } /** 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();