Created component wrapping the logic to determine what's the URL to redirect to for a ShortUrl

This commit is contained in:
Alejandro Celaya 2021-07-14 16:36:03 +02:00
parent 0af6ecbd34
commit d4cad337fc
7 changed files with 103 additions and 35 deletions

View file

@ -53,6 +53,7 @@ return [
ShortUrl\Resolver\PersistenceShortUrlRelationResolver::class => ConfigAbstractFactory::class,
ShortUrl\Helper\ShortUrlStringifier::class => ConfigAbstractFactory::class,
ShortUrl\Helper\ShortUrlTitleResolutionHelper::class => ConfigAbstractFactory::class,
ShortUrl\Helper\ShortUrlRedirectionBuilder::class => ConfigAbstractFactory::class,
ShortUrl\Transformer\ShortUrlDataTransformer::class => ConfigAbstractFactory::class,
Mercure\MercureUpdatesGenerator::class => ConfigAbstractFactory::class,
@ -117,6 +118,7 @@ return [
Action\RedirectAction::class => [
Service\ShortUrl\ShortUrlResolver::class,
Visit\VisitsTracker::class,
ShortUrl\Helper\ShortUrlRedirectionBuilder::class,
Options\TrackingOptions::class,
Util\RedirectResponseHelper::class,
'Logger_Shlink',
@ -124,6 +126,7 @@ return [
Action\PixelAction::class => [
Service\ShortUrl\ShortUrlResolver::class,
Visit\VisitsTracker::class,
ShortUrl\Helper\ShortUrlRedirectionBuilder::class,
Options\TrackingOptions::class,
'Logger_Shlink',
],
@ -137,6 +140,7 @@ return [
ShortUrl\Resolver\PersistenceShortUrlRelationResolver::class => ['em'],
ShortUrl\Helper\ShortUrlStringifier::class => ['config.url_shortener.domain', 'config.router.base_path'],
ShortUrl\Helper\ShortUrlTitleResolutionHelper::class => [Util\UrlValidator::class],
ShortUrl\Helper\ShortUrlRedirectionBuilder::class => [Options\TrackingOptions::class],
ShortUrl\Transformer\ShortUrlDataTransformer::class => [ShortUrl\Helper\ShortUrlStringifier::class],
Mercure\MercureUpdatesGenerator::class => [

View file

@ -5,8 +5,6 @@ declare(strict_types=1);
namespace Shlinkio\Shlink\Core\Action;
use Fig\Http\Message\RequestMethodInterface;
use GuzzleHttp\Psr7\Query;
use League\Uri\Uri;
use Mezzio\Router\Middleware\ImplicitHeadMiddleware;
use Psr\Http\Message\ResponseInterface;
use Psr\Http\Message\ServerRequestInterface;
@ -14,16 +12,15 @@ use Psr\Http\Server\MiddlewareInterface;
use Psr\Http\Server\RequestHandlerInterface;
use Psr\Log\LoggerInterface;
use Psr\Log\NullLogger;
use Shlinkio\Shlink\Core\Entity\ShortUrl;
use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException;
use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier;
use Shlinkio\Shlink\Core\Model\Visitor;
use Shlinkio\Shlink\Core\Options\TrackingOptions;
use Shlinkio\Shlink\Core\Service\ShortUrl\ShortUrlResolverInterface;
use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlRedirectionBuilderInterface;
use Shlinkio\Shlink\Core\Visit\VisitsTrackerInterface;
use function array_key_exists;
use function array_merge;
abstract class AbstractTrackingAction implements MiddlewareInterface, RequestMethodInterface
{
@ -32,6 +29,7 @@ abstract class AbstractTrackingAction implements MiddlewareInterface, RequestMet
public function __construct(
private ShortUrlResolverInterface $urlResolver,
private VisitsTrackerInterface $visitTracker,
private ShortUrlRedirectionBuilderInterface $redirectionBuilder,
private TrackingOptions $trackingOptions,
?LoggerInterface $logger = null
) {
@ -42,36 +40,24 @@ abstract class AbstractTrackingAction implements MiddlewareInterface, RequestMet
{
$identifier = ShortUrlIdentifier::fromRedirectRequest($request);
$query = $request->getQueryParams();
$disableTrackParam = $this->trackingOptions->getDisableTrackParam();
try {
$shortUrl = $this->urlResolver->resolveEnabledShortUrl($identifier);
if ($this->shouldTrackRequest($request, $query, $disableTrackParam)) {
if ($this->shouldTrackRequest($request, $query)) {
$this->visitTracker->track($shortUrl, Visitor::fromRequest($request));
}
return $this->createSuccessResp($this->buildUrlToRedirectTo($shortUrl, $query, $disableTrackParam));
return $this->createSuccessResp($this->redirectionBuilder->buildShortUrlRedirect($shortUrl, $query));
} catch (ShortUrlNotFoundException $e) {
$this->logger->warning('An error occurred while tracking short code. {e}', ['e' => $e]);
return $this->createErrorResp($request, $handler);
}
}
private function buildUrlToRedirectTo(ShortUrl $shortUrl, array $currentQuery, ?string $disableTrackParam): string
{
$uri = Uri::createFromString($shortUrl->getLongUrl());
$hardcodedQuery = Query::parse($uri->getQuery() ?? '');
if ($disableTrackParam !== null) {
unset($currentQuery[$disableTrackParam]);
}
$mergedQuery = array_merge($hardcodedQuery, $currentQuery);
return (string) (empty($mergedQuery) ? $uri : $uri->withQuery(Query::build($mergedQuery)));
}
private function shouldTrackRequest(ServerRequestInterface $request, array $query, ?string $disableTrackParam): bool
private function shouldTrackRequest(ServerRequestInterface $request, array $query): bool
{
$disableTrackParam = $this->trackingOptions->getDisableTrackParam();
$forwardedMethod = $request->getAttribute(ImplicitHeadMiddleware::FORWARDED_HTTP_METHOD_ATTRIBUTE);
if ($forwardedMethod === self::METHOD_HEAD) {
return false;

View file

@ -11,6 +11,7 @@ use Psr\Http\Server\RequestHandlerInterface;
use Psr\Log\LoggerInterface;
use Shlinkio\Shlink\Core\Options;
use Shlinkio\Shlink\Core\Service\ShortUrl\ShortUrlResolverInterface;
use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlRedirectionBuilderInterface;
use Shlinkio\Shlink\Core\Util\RedirectResponseHelperInterface;
use Shlinkio\Shlink\Core\Visit\VisitsTrackerInterface;
@ -19,11 +20,12 @@ class RedirectAction extends AbstractTrackingAction implements StatusCodeInterfa
public function __construct(
ShortUrlResolverInterface $urlResolver,
VisitsTrackerInterface $visitTracker,
ShortUrlRedirectionBuilderInterface $redirectionBuilder,
Options\TrackingOptions $trackingOptions,
private RedirectResponseHelperInterface $redirectResponseHelper,
?LoggerInterface $logger = null
) {
parent::__construct($urlResolver, $visitTracker, $trackingOptions, $logger);
parent::__construct($urlResolver, $visitTracker, $redirectionBuilder, $trackingOptions, $logger);
}
protected function createSuccessResp(string $longUrl): Response

View file

@ -0,0 +1,50 @@
<?php
declare(strict_types=1);
namespace Shlinkio\Shlink\Core\ShortUrl\Helper;
use GuzzleHttp\Psr7\Query;
use League\Uri\Uri;
use Shlinkio\Shlink\Core\Entity\ShortUrl;
use Shlinkio\Shlink\Core\Options\TrackingOptions;
use function array_merge;
use function sprintf;
class ShortUrlRedirectionBuilder implements ShortUrlRedirectionBuilderInterface
{
public function __construct(private TrackingOptions $trackingOptions)
{
}
public function buildShortUrlRedirect(ShortUrl $shortUrl, array $currentQuery, ?string $extraPath = null): string
{
$uri = Uri::createFromString($shortUrl->getLongUrl());
return $uri
->withQuery($this->resolveQuery($uri, $currentQuery))
->withPath($this->resolvePath($uri, $extraPath))
->__toString();
}
private function resolveQuery(Uri $uri, array $currentQuery): string
{
$hardcodedQuery = Query::parse($uri->getQuery() ?? '');
$disableTrackParam = $this->trackingOptions->getDisableTrackParam();
if ($disableTrackParam !== null) {
unset($currentQuery[$disableTrackParam]);
}
$mergedQuery = array_merge($hardcodedQuery, $currentQuery);
return empty($mergedQuery) ? '' : Query::build($mergedQuery);
}
private function resolvePath(Uri $uri, ?string $extraPath): string
{
$hardcodedPath = $uri->getPath();
return $extraPath === null ? $hardcodedPath : sprintf('%s%s', $hardcodedPath, $extraPath);
}
}

View file

@ -0,0 +1,12 @@
<?php
declare(strict_types=1);
namespace Shlinkio\Shlink\Core\ShortUrl\Helper;
use Shlinkio\Shlink\Core\Entity\ShortUrl;
interface ShortUrlRedirectionBuilderInterface
{
public function buildShortUrlRedirect(ShortUrl $shortUrl, array $currentQuery, ?string $extraPath = null): string;
}

View file

@ -16,6 +16,7 @@ use Shlinkio\Shlink\Core\Entity\ShortUrl;
use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier;
use Shlinkio\Shlink\Core\Options\TrackingOptions;
use Shlinkio\Shlink\Core\Service\ShortUrl\ShortUrlResolverInterface;
use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlRedirectionBuilderInterface;
use Shlinkio\Shlink\Core\Visit\VisitsTracker;
class PixelActionTest extends TestCase
@ -31,9 +32,13 @@ class PixelActionTest extends TestCase
$this->urlResolver = $this->prophesize(ShortUrlResolverInterface::class);
$this->visitTracker = $this->prophesize(VisitsTracker::class);
$redirectBuilder = $this->prophesize(ShortUrlRedirectionBuilderInterface::class);
$redirectBuilder->buildShortUrlRedirect(Argument::cetera())->willReturn('http://domain.com/foo/bar');
$this->action = new PixelAction(
$this->urlResolver->reveal(),
$this->visitTracker->reveal(),
$redirectBuilder->reveal(),
new TrackingOptions(),
);
}

View file

@ -19,6 +19,7 @@ use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException;
use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier;
use Shlinkio\Shlink\Core\Options;
use Shlinkio\Shlink\Core\Service\ShortUrl\ShortUrlResolverInterface;
use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlRedirectionBuilderInterface;
use Shlinkio\Shlink\Core\Util\RedirectResponseHelperInterface;
use Shlinkio\Shlink\Core\Visit\VisitsTrackerInterface;
@ -28,6 +29,8 @@ class RedirectActionTest extends TestCase
{
use ProphecyTrait;
private const LONG_URL = 'https://domain.com/foo/bar?some=thing';
private RedirectAction $action;
private ObjectProphecy $urlResolver;
private ObjectProphecy $visitTracker;
@ -39,9 +42,13 @@ class RedirectActionTest extends TestCase
$this->visitTracker = $this->prophesize(VisitsTrackerInterface::class);
$this->redirectRespHelper = $this->prophesize(RedirectResponseHelperInterface::class);
$redirectBuilder = $this->prophesize(ShortUrlRedirectionBuilderInterface::class);
$redirectBuilder->buildShortUrlRedirect(Argument::cetera())->willReturn(self::LONG_URL);
$this->action = new RedirectAction(
$this->urlResolver->reveal(),
$this->visitTracker->reveal(),
$redirectBuilder->reveal(),
new Options\TrackingOptions(['disableTrackParam' => 'foobar']),
$this->redirectRespHelper->reveal(),
);
@ -51,17 +58,17 @@ class RedirectActionTest extends TestCase
* @test
* @dataProvider provideQueries
*/
public function redirectionIsPerformedToLongUrl(string $expectedUrl, array $query): void
public function redirectionIsPerformedToLongUrl(array $query): void
{
$shortCode = 'abc123';
$shortUrl = ShortUrl::withLongUrl('http://domain.com/foo/bar?some=thing');
$shortUrl = ShortUrl::withLongUrl(self::LONG_URL);
$shortCodeToUrl = $this->urlResolver->resolveEnabledShortUrl(
new ShortUrlIdentifier($shortCode, ''),
)->willReturn($shortUrl);
$track = $this->visitTracker->track(Argument::cetera())->will(function (): void {
});
$expectedResp = new Response\RedirectResponse($expectedUrl);
$buildResp = $this->redirectRespHelper->buildRedirectResponse($expectedUrl)->willReturn($expectedResp);
$expectedResp = new Response\RedirectResponse(self::LONG_URL);
$buildResp = $this->redirectRespHelper->buildRedirectResponse(self::LONG_URL)->willReturn($expectedResp);
$request = (new ServerRequest())->withAttribute('shortCode', $shortCode)->withQueryParams($query);
$response = $this->action->process($request, $this->prophesize(RequestHandlerInterface::class)->reveal());
@ -74,12 +81,14 @@ class RedirectActionTest extends TestCase
public function provideQueries(): iterable
{
yield ['http://domain.com/foo/bar?some=thing', []];
yield ['http://domain.com/foo/bar?some=thing', ['foobar' => 'notrack']];
yield ['http://domain.com/foo/bar?some=thing&else', ['else' => null]];
yield ['http://domain.com/foo/bar?some=thing&foo=bar', ['foo' => 'bar']];
yield ['http://domain.com/foo/bar?some=overwritten&foo=bar', ['foo' => 'bar', 'some' => 'overwritten']];
yield ['http://domain.com/foo/bar?some=overwritten', ['foobar' => 'notrack', 'some' => 'overwritten']];
yield [[]];
yield [['foobar' => 'notrack']];
yield [['foobar' => 'barfoo']];
yield [['foobar' => null]];
// yield ['http://domain.com/foo/bar?some=thing&else', ['else' => null]];
// yield ['http://domain.com/foo/bar?some=thing&foo=bar', ['foo' => 'bar']];
// yield ['http://domain.com/foo/bar?some=overwritten&foo=bar', ['foo' => 'bar', 'some' => 'overwritten']];
// yield ['http://domain.com/foo/bar?some=overwritten', ['foobar' => 'notrack', 'some' => 'overwritten']];
}
/** @test */
@ -104,13 +113,13 @@ class RedirectActionTest extends TestCase
public function trackingIsDisabledWhenRequestIsForwardedFromHead(): void
{
$shortCode = 'abc123';
$shortUrl = ShortUrl::withLongUrl('http://domain.com/foo/bar?some=thing');
$shortUrl = ShortUrl::withLongUrl(self::LONG_URL);
$this->urlResolver->resolveEnabledShortUrl(new ShortUrlIdentifier($shortCode, ''))->willReturn($shortUrl);
$track = $this->visitTracker->track(Argument::cetera())->will(function (): void {
});
$buildResp = $this->redirectRespHelper->buildRedirectResponse(
'http://domain.com/foo/bar?some=thing',
)->willReturn(new Response\RedirectResponse(''));
$buildResp = $this->redirectRespHelper->buildRedirectResponse(self::LONG_URL)->willReturn(
new Response\RedirectResponse(''),
);
$request = (new ServerRequest())->withAttribute('shortCode', $shortCode)
->withAttribute(