diff --git a/module/CLI/src/Command/Shortcode/ResolveUrlCommand.php b/module/CLI/src/Command/Shortcode/ResolveUrlCommand.php index fcf62fbe..2bd87a89 100644 --- a/module/CLI/src/Command/Shortcode/ResolveUrlCommand.php +++ b/module/CLI/src/Command/Shortcode/ResolveUrlCommand.php @@ -66,8 +66,10 @@ class ResolveUrlCommand extends Command $shortCode = $input->getArgument('shortCode'); try { - $longUrl = $this->urlShortener->shortCodeToUrl($shortCode); - $output->writeln(\sprintf('%s %s', $this->translator->translate('Long URL:'), $longUrl)); + $url = $this->urlShortener->shortCodeToUrl($shortCode); + $output->writeln( + \sprintf('%s %s', $this->translator->translate('Long URL:'), $url->getLongUrl()) + ); } catch (InvalidShortCodeException $e) { $io->error( \sprintf($this->translator->translate('Provided short code "%s" has an invalid format.'), $shortCode) diff --git a/module/CLI/test/Command/Shortcode/ResolveUrlCommandTest.php b/module/CLI/test/Command/Shortcode/ResolveUrlCommandTest.php index d1bc50a3..dfa2e483 100644 --- a/module/CLI/test/Command/Shortcode/ResolveUrlCommandTest.php +++ b/module/CLI/test/Command/Shortcode/ResolveUrlCommandTest.php @@ -6,6 +6,7 @@ namespace ShlinkioTest\Shlink\CLI\Command\Shortcode; use PHPUnit\Framework\TestCase; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\CLI\Command\Shortcode\ResolveUrlCommand; +use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException; use Shlinkio\Shlink\Core\Exception\InvalidShortCodeException; use Shlinkio\Shlink\Core\Service\UrlShortener; @@ -41,7 +42,8 @@ class ResolveUrlCommandTest extends TestCase { $shortCode = 'abc123'; $expectedUrl = 'http://domain.com/foo/bar'; - $this->urlShortener->shortCodeToUrl($shortCode)->willReturn($expectedUrl) + $shortUrl = (new ShortUrl())->setLongUrl($expectedUrl); + $this->urlShortener->shortCodeToUrl($shortCode)->willReturn($shortUrl) ->shouldBeCalledTimes(1); $this->commandTester->execute([ diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index 58def9ec..8a696525 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -42,7 +42,6 @@ return [ Service\UrlShortener::class => [ 'httpClient', 'em', - Cache::class, 'config.url_shortener.validate_url', 'config.url_shortener.shortcode_chars', ], diff --git a/module/Core/src/Action/AbstractTrackingAction.php b/module/Core/src/Action/AbstractTrackingAction.php index 147e31b6..cfb4223c 100644 --- a/module/Core/src/Action/AbstractTrackingAction.php +++ b/module/Core/src/Action/AbstractTrackingAction.php @@ -65,14 +65,14 @@ abstract class AbstractTrackingAction implements MiddlewareInterface $disableTrackParam = $this->appOptions->getDisableTrackParam(); try { - $longUrl = $this->urlShortener->shortCodeToUrl($shortCode); + $url = $this->urlShortener->shortCodeToUrl($shortCode); // Track visit to this short code if ($disableTrackParam === null || ! \array_key_exists($disableTrackParam, $query)) { $this->visitTracker->track($shortCode, $request); } - return $this->createResp($longUrl); + return $this->createResp($url->getLongUrl()); } catch (InvalidShortCodeException | EntityDoesNotExistException $e) { $this->logger->warning('An error occurred while tracking short code.' . PHP_EOL . $e); return $this->buildErrorResponse($request, $handler); diff --git a/module/Core/src/Action/PreviewAction.php b/module/Core/src/Action/PreviewAction.php index 83748a3b..9f95e884 100644 --- a/module/Core/src/Action/PreviewAction.php +++ b/module/Core/src/Action/PreviewAction.php @@ -60,7 +60,7 @@ class PreviewAction implements MiddlewareInterface try { $url = $this->urlShortener->shortCodeToUrl($shortCode); - $imagePath = $this->previewGenerator->generatePreview($url); + $imagePath = $this->previewGenerator->generatePreview($url->getLongUrl()); return $this->generateImageResponse($imagePath); } catch (InvalidShortCodeException | EntityDoesNotExistException | PreviewGenerationException $e) { $this->logger->warning('An error occurred while generating preview image.' . PHP_EOL . $e); diff --git a/module/Core/src/Service/UrlShortener.php b/module/Core/src/Service/UrlShortener.php index e1696adc..d5bf8cc2 100644 --- a/module/Core/src/Service/UrlShortener.php +++ b/module/Core/src/Service/UrlShortener.php @@ -5,7 +5,6 @@ namespace Shlinkio\Shlink\Core\Service; use Cocur\Slugify\Slugify; use Cocur\Slugify\SlugifyInterface; -use Doctrine\Common\Cache\Cache; use Doctrine\ORM\EntityManagerInterface; use GuzzleHttp\ClientInterface; use GuzzleHttp\Exception\GuzzleException; @@ -37,10 +36,6 @@ class UrlShortener implements UrlShortenerInterface * @var string */ private $chars; - /** - * @var Cache - */ - private $cache; /** * @var SlugifyInterface */ @@ -53,14 +48,12 @@ class UrlShortener implements UrlShortenerInterface public function __construct( ClientInterface $httpClient, EntityManagerInterface $em, - Cache $cache, $urlValidationEnabled, $chars = self::DEFAULT_CHARS, SlugifyInterface $slugger = null ) { $this->httpClient = $httpClient; $this->em = $em; - $this->cache = $cache; $this->urlValidationEnabled = $urlValidationEnabled; $this->chars = empty($chars) ? self::DEFAULT_CHARS : $chars; $this->slugger = $slugger ?: new Slugify(); @@ -192,19 +185,11 @@ class UrlShortener implements UrlShortenerInterface /** * Tries to find the mapped URL for provided short code. Returns null if not found * - * @param string $shortCode - * @return string * @throws InvalidShortCodeException * @throws EntityDoesNotExistException */ - public function shortCodeToUrl(string $shortCode): string + public function shortCodeToUrl(string $shortCode): ShortUrl { - $cacheKey = sprintf('%s_longUrl', $shortCode); - // Check if the short code => URL map is already cached - if ($this->cache->contains($cacheKey)) { - return $this->cache->fetch($cacheKey); - } - // Validate short code format if (! preg_match('|[' . $this->chars . ']+|', $shortCode)) { throw InvalidShortCodeException::fromCharset($shortCode, $this->chars); @@ -219,9 +204,6 @@ class UrlShortener implements UrlShortenerInterface ]); } - // Cache the shortcode - $url = $shortUrl->getOriginalUrl(); - $this->cache->save($cacheKey, $url); - return $url; + return $shortUrl; } } diff --git a/module/Core/src/Service/UrlShortenerInterface.php b/module/Core/src/Service/UrlShortenerInterface.php index 9dd354d5..7b97af91 100644 --- a/module/Core/src/Service/UrlShortenerInterface.php +++ b/module/Core/src/Service/UrlShortenerInterface.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Service; use Psr\Http\Message\UriInterface; +use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException; use Shlinkio\Shlink\Core\Exception\InvalidShortCodeException; use Shlinkio\Shlink\Core\Exception\InvalidUrlException; @@ -21,7 +22,6 @@ interface UrlShortenerInterface * @param \DateTime|null $validUntil * @param string|null $customSlug * @param int|null $maxVisits - * @return string * @throws NonUniqueSlugException * @throws InvalidUrlException * @throws RuntimeException @@ -38,10 +38,8 @@ interface UrlShortenerInterface /** * Tries to find the mapped URL for provided short code. Returns null if not found * - * @param string $shortCode - * @return string * @throws InvalidShortCodeException * @throws EntityDoesNotExistException */ - public function shortCodeToUrl(string $shortCode): string; + public function shortCodeToUrl(string $shortCode): ShortUrl; } diff --git a/module/Core/test/Action/PixelActionTest.php b/module/Core/test/Action/PixelActionTest.php index 1a7f7296..f827b0d1 100644 --- a/module/Core/test/Action/PixelActionTest.php +++ b/module/Core/test/Action/PixelActionTest.php @@ -9,6 +9,7 @@ use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\Common\Response\PixelResponse; use Shlinkio\Shlink\Core\Action\PixelAction; use Shlinkio\Shlink\Core\Action\RedirectAction; +use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Options\AppOptions; use Shlinkio\Shlink\Core\Service\UrlShortener; use Shlinkio\Shlink\Core\Service\VisitsTracker; @@ -48,8 +49,9 @@ class PixelActionTest extends TestCase public function imageIsReturned() { $shortCode = 'abc123'; - $this->urlShortener->shortCodeToUrl($shortCode)->willReturn('http://domain.com/foo/bar') - ->shouldBeCalledTimes(1); + $this->urlShortener->shortCodeToUrl($shortCode)->willReturn( + (new ShortUrl())->setLongUrl('http://domain.com/foo/bar') + )->shouldBeCalledTimes(1); $this->visitTracker->track(Argument::cetera())->willReturn(null) ->shouldBeCalledTimes(1); diff --git a/module/Core/test/Action/PreviewActionTest.php b/module/Core/test/Action/PreviewActionTest.php index abd4dc35..d1c0a23d 100644 --- a/module/Core/test/Action/PreviewActionTest.php +++ b/module/Core/test/Action/PreviewActionTest.php @@ -10,6 +10,7 @@ use Prophecy\Prophecy\ObjectProphecy; use Psr\Http\Server\RequestHandlerInterface; use Shlinkio\Shlink\Common\Service\PreviewGenerator; use Shlinkio\Shlink\Core\Action\PreviewAction; +use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException; use Shlinkio\Shlink\Core\Exception\InvalidShortCodeException; use Shlinkio\Shlink\Core\Service\UrlShortener; @@ -64,8 +65,9 @@ class PreviewActionTest extends TestCase { $shortCode = 'abc123'; $url = 'foobar.com'; + $shortUrl = (new ShortUrl())->setLongUrl($url); $path = __FILE__; - $this->urlShortener->shortCodeToUrl($shortCode)->willReturn($url)->shouldBeCalledTimes(1); + $this->urlShortener->shortCodeToUrl($shortCode)->willReturn($shortUrl)->shouldBeCalledTimes(1); $this->previewGenerator->generatePreview($url)->willReturn($path)->shouldBeCalledTimes(1); $resp = $this->action->process( diff --git a/module/Core/test/Action/QrCodeActionTest.php b/module/Core/test/Action/QrCodeActionTest.php index 2d58c8c9..4e24303b 100644 --- a/module/Core/test/Action/QrCodeActionTest.php +++ b/module/Core/test/Action/QrCodeActionTest.php @@ -10,6 +10,7 @@ use Prophecy\Prophecy\ObjectProphecy; use Psr\Http\Server\RequestHandlerInterface; use Shlinkio\Shlink\Common\Response\QrCodeResponse; use Shlinkio\Shlink\Core\Action\QrCodeAction; +use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException; use Shlinkio\Shlink\Core\Exception\InvalidShortCodeException; use Shlinkio\Shlink\Core\Service\UrlShortener; @@ -83,7 +84,8 @@ class QrCodeActionTest extends TestCase public function aCorrectRequestReturnsTheQrCodeResponse() { $shortCode = 'abc123'; - $this->urlShortener->shortCodeToUrl($shortCode)->willReturn('')->shouldBeCalledTimes(1); + $this->urlShortener->shortCodeToUrl($shortCode)->willReturn((new ShortUrl())->setLongUrl('')) + ->shouldBeCalledTimes(1); $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 88b44d07..0c83f8fe 100644 --- a/module/Core/test/Action/RedirectActionTest.php +++ b/module/Core/test/Action/RedirectActionTest.php @@ -9,6 +9,7 @@ use Prophecy\Prophecy\MethodProphecy; use Prophecy\Prophecy\ObjectProphecy; use Psr\Http\Server\RequestHandlerInterface; use Shlinkio\Shlink\Core\Action\RedirectAction; +use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException; use Shlinkio\Shlink\Core\Options\AppOptions; use Shlinkio\Shlink\Core\Service\UrlShortener; @@ -51,7 +52,8 @@ class RedirectActionTest extends TestCase { $shortCode = 'abc123'; $expectedUrl = 'http://domain.com/foo/bar'; - $this->urlShortener->shortCodeToUrl($shortCode)->willReturn($expectedUrl) + $shortUrl = (new ShortUrl())->setLongUrl($expectedUrl); + $this->urlShortener->shortCodeToUrl($shortCode)->willReturn($shortUrl) ->shouldBeCalledTimes(1); $this->visitTracker->track(Argument::cetera())->willReturn(null) ->shouldBeCalledTimes(1); @@ -93,7 +95,8 @@ class RedirectActionTest extends TestCase { $shortCode = 'abc123'; $expectedUrl = 'http://domain.com/foo/bar'; - $this->urlShortener->shortCodeToUrl($shortCode)->willReturn($expectedUrl) + $shortUrl = (new ShortUrl())->setLongUrl($expectedUrl); + $this->urlShortener->shortCodeToUrl($shortCode)->willReturn($shortUrl) ->shouldBeCalledTimes(1); $this->visitTracker->track(Argument::cetera())->willReturn(null) ->shouldNotBeCalled(); diff --git a/module/Core/test/Service/UrlShortenerTest.php b/module/Core/test/Service/UrlShortenerTest.php index 6f78c70c..5ba33612 100644 --- a/module/Core/test/Service/UrlShortenerTest.php +++ b/module/Core/test/Service/UrlShortenerTest.php @@ -4,8 +4,6 @@ declare(strict_types=1); namespace ShlinkioTest\Shlink\Core\Service; use Cocur\Slugify\SlugifyInterface; -use Doctrine\Common\Cache\ArrayCache; -use Doctrine\Common\Cache\Cache; use Doctrine\Common\Persistence\ObjectRepository; use Doctrine\DBAL\Connection; use Doctrine\ORM\EntityManagerInterface; @@ -37,10 +35,6 @@ class UrlShortenerTest extends TestCase * @var ObjectProphecy */ protected $httpClient; - /** - * @var Cache - */ - protected $cache; /** * @var ObjectProphecy */ @@ -66,7 +60,6 @@ class UrlShortenerTest extends TestCase $repo->findOneBy(Argument::any())->willReturn(null); $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal()); - $this->cache = new ArrayCache(); $this->slugger = $this->prophesize(SlugifyInterface::class); $this->setUrlShortener(false); @@ -80,7 +73,6 @@ class UrlShortenerTest extends TestCase $this->urlShortener = new UrlShortener( $this->httpClient->reveal(), $this->em->reveal(), - $this->cache, $urlValidationEnabled, UrlShortener::DEFAULT_CHARS, $this->slugger->reveal() @@ -205,10 +197,8 @@ class UrlShortenerTest extends TestCase $repo->findOneByShortCode($shortCode)->willReturn($shortUrl); $this->em->getRepository(ShortUrl::class)->willReturn($repo->reveal()); - $this->assertFalse($this->cache->contains($shortCode . '_longUrl')); $url = $this->urlShortener->shortCodeToUrl($shortCode); - $this->assertEquals($shortUrl->getOriginalUrl(), $url); - $this->assertTrue($this->cache->contains($shortCode . '_longUrl')); + $this->assertSame($shortUrl, $url); } /** @@ -219,18 +209,4 @@ class UrlShortenerTest extends TestCase { $this->urlShortener->shortCodeToUrl('&/('); } - - /** - * @test - */ - public function cachedShortCodeDoesNotHitDatabase() - { - $shortCode = '12C1c'; - $expectedUrl = 'expected_url'; - $this->cache->save($shortCode . '_longUrl', $expectedUrl); - $this->em->getRepository(ShortUrl::class)->willReturn(null)->shouldBeCalledTimes(0); - - $url = $this->urlShortener->shortCodeToUrl($shortCode); - $this->assertEquals($expectedUrl, $url); - } } diff --git a/module/Rest/config/dependencies.config.php b/module/Rest/config/dependencies.config.php index 42147707..92c991e1 100644 --- a/module/Rest/config/dependencies.config.php +++ b/module/Rest/config/dependencies.config.php @@ -59,7 +59,11 @@ return [ 'Logger_Shlink', ], Action\ShortCode\EditShortCodeAction::class => [Service\ShortUrlService::class, 'translator', 'Logger_Shlink',], - Action\ShortCode\ResolveUrlAction::class => [Service\UrlShortener::class, 'translator'], + Action\ShortCode\ResolveUrlAction::class => [ + Service\UrlShortener::class, + 'translator', + 'config.url_shortener.domain', + ], Action\Visit\GetVisitsAction::class => [Service\VisitsTracker::class, 'translator', 'Logger_Shlink'], Action\ShortCode\ListShortCodesAction::class => [ Service\ShortUrlService::class, diff --git a/module/Rest/src/Action/ShortCode/ResolveUrlAction.php b/module/Rest/src/Action/ShortCode/ResolveUrlAction.php index 8a7f2ba7..d847c4d9 100644 --- a/module/Rest/src/Action/ShortCode/ResolveUrlAction.php +++ b/module/Rest/src/Action/ShortCode/ResolveUrlAction.php @@ -9,6 +9,7 @@ use Psr\Log\LoggerInterface; use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException; use Shlinkio\Shlink\Core\Exception\InvalidShortCodeException; use Shlinkio\Shlink\Core\Service\UrlShortenerInterface; +use Shlinkio\Shlink\Core\Transformer\ShortUrlDataTransformer; use Shlinkio\Shlink\Rest\Action\AbstractRestAction; use Shlinkio\Shlink\Rest\Util\RestUtils; use Zend\Diactoros\Response\JsonResponse; @@ -27,15 +28,21 @@ class ResolveUrlAction extends AbstractRestAction * @var TranslatorInterface */ private $translator; + /** + * @var array + */ + private $domainConfig; public function __construct( UrlShortenerInterface $urlShortener, TranslatorInterface $translator, + array $domainConfig, LoggerInterface $logger = null ) { parent::__construct($logger); $this->urlShortener = $urlShortener; $this->translator = $translator; + $this->domainConfig = $domainConfig; } /** @@ -46,17 +53,16 @@ class ResolveUrlAction extends AbstractRestAction public function handle(Request $request): Response { $shortCode = $request->getAttribute('shortCode'); + $transformer = new ShortUrlDataTransformer($this->domainConfig); try { - $longUrl = $this->urlShortener->shortCodeToUrl($shortCode); - return new JsonResponse([ - 'longUrl' => $longUrl, - ]); + $url = $this->urlShortener->shortCodeToUrl($shortCode); + return new JsonResponse($transformer->transform($url)); } catch (InvalidShortCodeException $e) { $this->logger->warning('Provided short code with invalid format.' . PHP_EOL . $e); return new JsonResponse([ 'error' => RestUtils::getRestErrorCodeFromException($e), - 'message' => sprintf( + 'message' => \sprintf( $this->translator->translate('Provided short code "%s" has an invalid format'), $shortCode ), @@ -65,7 +71,7 @@ class ResolveUrlAction extends AbstractRestAction $this->logger->warning('Provided short code couldn\'t be found.' . PHP_EOL . $e); return new JsonResponse([ 'error' => RestUtils::INVALID_ARGUMENT_ERROR, - 'message' => sprintf($this->translator->translate('No URL found for short code "%s"'), $shortCode), + 'message' => \sprintf($this->translator->translate('No URL found for short code "%s"'), $shortCode), ], self::STATUS_NOT_FOUND); } catch (\Exception $e) { $this->logger->error('Unexpected error while resolving the URL behind a short code.' . PHP_EOL . $e); diff --git a/module/Rest/test/Action/ShortCode/ResolveUrlActionTest.php b/module/Rest/test/Action/ShortCode/ResolveUrlActionTest.php index 3761604e..bde3e89a 100644 --- a/module/Rest/test/Action/ShortCode/ResolveUrlActionTest.php +++ b/module/Rest/test/Action/ShortCode/ResolveUrlActionTest.php @@ -5,6 +5,7 @@ namespace ShlinkioTest\Shlink\Rest\Action\ShortCode; use PHPUnit\Framework\TestCase; use Prophecy\Prophecy\ObjectProphecy; +use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Exception\EntityDoesNotExistException; use Shlinkio\Shlink\Core\Exception\InvalidShortCodeException; use Shlinkio\Shlink\Core\Service\UrlShortener; @@ -27,7 +28,7 @@ class ResolveUrlActionTest extends TestCase public function setUp() { $this->urlShortener = $this->prophesize(UrlShortener::class); - $this->action = new ResolveUrlAction($this->urlShortener->reveal(), Translator::factory([])); + $this->action = new ResolveUrlAction($this->urlShortener->reveal(), Translator::factory([]), []); } /** @@ -51,8 +52,9 @@ class ResolveUrlActionTest extends TestCase public function correctShortCodeReturnsSuccess() { $shortCode = 'abc123'; - $this->urlShortener->shortCodeToUrl($shortCode)->willReturn('http://domain.com/foo/bar') - ->shouldBeCalledTimes(1); + $this->urlShortener->shortCodeToUrl($shortCode)->willReturn( + (new ShortUrl())->setLongUrl('http://domain.com/foo/bar') + )->shouldBeCalledTimes(1); $request = ServerRequestFactory::fromGlobals()->withAttribute('shortCode', $shortCode); $response = $this->action->handle($request);