Added forward of query string from short URLs to long one

This commit is contained in:
Alejandro Celaya 2019-11-13 21:04:44 +01:00
parent 3b9221c7d2
commit 705dc2ec39
2 changed files with 41 additions and 29 deletions

View file

@ -10,14 +10,19 @@ 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\EntityDoesNotExistException;
use Shlinkio\Shlink\Core\Exception\InvalidShortCodeException;
use Shlinkio\Shlink\Core\Model\Visitor;
use Shlinkio\Shlink\Core\Options\AppOptions;
use Shlinkio\Shlink\Core\Service\UrlShortenerInterface;
use Shlinkio\Shlink\Core\Service\VisitsTrackerInterface;
use Zend\Diactoros\Uri;
use function array_key_exists;
use function array_merge;
use function GuzzleHttp\Psr7\parse_query;
use function http_build_query;
abstract class AbstractTrackingAction implements MiddlewareInterface
{
@ -66,13 +71,25 @@ abstract class AbstractTrackingAction implements MiddlewareInterface
$this->visitTracker->track($shortCode, Visitor::fromRequest($request));
}
return $this->createSuccessResp($url->getLongUrl());
return $this->createSuccessResp($this->buildUrlToRedirectTo($url, $query, $disableTrackParam));
} catch (InvalidShortCodeException | EntityDoesNotExistException $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 = new Uri($shortUrl->getLongUrl());
$hardcodedQuery = parse_query($uri->getQuery());
if ($disableTrackParam !== null) {
unset($currentQuery[$disableTrackParam]);
}
$mergedQuery = array_merge($hardcodedQuery, $currentQuery);
return (string) $uri->withQuery(http_build_query($mergedQuery));
}
abstract protected function createSuccessResp(string $longUrl): ResponseInterface;
abstract protected function createErrorResp(

View file

@ -17,6 +17,8 @@ use Shlinkio\Shlink\Core\Service\VisitsTracker;
use Zend\Diactoros\Response;
use Zend\Diactoros\ServerRequest;
use function array_key_exists;
class RedirectActionTest extends TestCase
{
/** @var RedirectAction */
@ -38,23 +40,36 @@ class RedirectActionTest extends TestCase
);
}
/** @test */
public function redirectionIsPerformedToLongUrl(): void
/**
* @test
* @dataProvider provideQueries
*/
public function redirectionIsPerformedToLongUrl(string $expectedUrl, array $query): void
{
$shortCode = 'abc123';
$expectedUrl = 'http://domain.com/foo/bar';
$shortUrl = new ShortUrl($expectedUrl);
$this->urlShortener->shortCodeToUrl($shortCode, '')->willReturn($shortUrl)
->shouldBeCalledOnce();
$this->visitTracker->track(Argument::cetera())->shouldBeCalledOnce();
$shortUrl = new ShortUrl('http://domain.com/foo/bar?some=thing');
$shortCodeToUrl = $this->urlShortener->shortCodeToUrl($shortCode, '')->willReturn($shortUrl);
$track = $this->visitTracker->track(Argument::cetera())->will(function () {
});
$request = (new ServerRequest())->withAttribute('shortCode', $shortCode);
$request = (new ServerRequest())->withAttribute('shortCode', $shortCode)->withQueryParams($query);
$response = $this->action->process($request, $this->prophesize(RequestHandlerInterface::class)->reveal());
$this->assertInstanceOf(Response\RedirectResponse::class, $response);
$this->assertEquals(302, $response->getStatusCode());
$this->assertTrue($response->hasHeader('Location'));
$this->assertEquals($expectedUrl, $response->getHeaderLine('Location'));
$shortCodeToUrl->shouldHaveBeenCalledOnce();
$track->shouldHaveBeenCalledTimes(array_key_exists('foobar', $query) ? 0 : 1);
}
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&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 */
@ -73,24 +88,4 @@ class RedirectActionTest extends TestCase
$handle->shouldHaveBeenCalledOnce();
}
/** @test */
public function visitIsNotTrackedIfDisableParamIsProvided(): void
{
$shortCode = 'abc123';
$expectedUrl = 'http://domain.com/foo/bar';
$shortUrl = new ShortUrl($expectedUrl);
$this->urlShortener->shortCodeToUrl($shortCode, '')->willReturn($shortUrl)
->shouldBeCalledOnce();
$this->visitTracker->track(Argument::cetera())->shouldNotBeCalled();
$request = (new ServerRequest())->withAttribute('shortCode', $shortCode)
->withQueryParams(['foobar' => true]);
$response = $this->action->process($request, $this->prophesize(RequestHandlerInterface::class)->reveal());
$this->assertInstanceOf(Response\RedirectResponse::class, $response);
$this->assertEquals(302, $response->getStatusCode());
$this->assertTrue($response->hasHeader('Location'));
$this->assertEquals($expectedUrl, $response->getHeaderLine('Location'));
}
}