Add logic to dynamically resolve the long URL to redirect to based on requesting device

This commit is contained in:
Alejandro Celaya
2023-01-21 11:15:38 +01:00
parent 237fb95b4b
commit b1b67c497e
7 changed files with 109 additions and 36 deletions

View File

@@ -74,7 +74,7 @@
"phpunit/phpunit": "^9.5", "phpunit/phpunit": "^9.5",
"roave/security-advisories": "dev-master", "roave/security-advisories": "dev-master",
"shlinkio/php-coding-standard": "~2.3.0", "shlinkio/php-coding-standard": "~2.3.0",
"shlinkio/shlink-test-utils": "^3.3", "shlinkio/shlink-test-utils": "^3.4",
"symfony/var-dumper": "^6.1", "symfony/var-dumper": "^6.1",
"veewee/composer-run-parallel": "^1.1" "veewee/composer-run-parallel": "^1.1"
}, },
@@ -97,7 +97,8 @@
"ShlinkioApiTest\\Shlink\\Rest\\": "module/Rest/test-api", "ShlinkioApiTest\\Shlink\\Rest\\": "module/Rest/test-api",
"ShlinkioDbTest\\Shlink\\Rest\\": "module/Rest/test-db", "ShlinkioDbTest\\Shlink\\Rest\\": "module/Rest/test-db",
"ShlinkioTest\\Shlink\\Core\\": "module/Core/test", "ShlinkioTest\\Shlink\\Core\\": "module/Core/test",
"ShlinkioDbTest\\Shlink\\Core\\": "module/Core/test-db" "ShlinkioDbTest\\Shlink\\Core\\": "module/Core/test-db",
"ShlinkioApiTest\\Shlink\\Core\\": "module/Core/test-api"
}, },
"files": [ "files": [
"config/test/constants.php" "config/test/constants.php"

View File

@@ -6,3 +6,10 @@ namespace ShlinkioTest\Shlink;
const API_TESTS_HOST = '127.0.0.1'; const API_TESTS_HOST = '127.0.0.1';
const API_TESTS_PORT = 9999; const API_TESTS_PORT = 9999;
const ANDROID_USER_AGENT = 'Mozilla/5.0 (Linux; Android 13) AppleWebKit/537.36 (KHTML, like Gecko) '
. 'Chrome/109.0.5414.86 Mobile Safari/537.36';
const IOS_USER_AGENT = 'Mozilla/5.0 (iPhone; CPU iPhone OS 16_2 like Mac OS X) AppleWebKit/605.1.15 '
. '(KHTML, like Gecko) FxiOS/109.0 Mobile/15E148 Safari/605.1.15';
const DESKTOP_USER_AGENT = 'Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like '
. 'Gecko) Chrome/109.0.0.0 Safari/537.36 Edg/109.0.1518.61';

View File

@@ -12,7 +12,7 @@ class DeviceLongUrl extends AbstractEntity
{ {
private function __construct( private function __construct(
private readonly ShortUrl $shortUrl, // No need to read this field. It's used by doctrine private readonly ShortUrl $shortUrl, // No need to read this field. It's used by doctrine
private readonly DeviceType $deviceType, public readonly DeviceType $deviceType,
private string $longUrl, private string $longUrl,
) { ) {
} }
@@ -27,11 +27,6 @@ class DeviceLongUrl extends AbstractEntity
return $this->longUrl; return $this->longUrl;
} }
public function deviceType(): DeviceType
{
return $this->deviceType;
}
public function updateLongUrl(string $longUrl): void public function updateLongUrl(string $longUrl): void
{ {
$this->longUrl = $longUrl; $this->longUrl = $longUrl;

View File

@@ -12,6 +12,7 @@ use Doctrine\Common\Collections\Selectable;
use Shlinkio\Shlink\Common\Entity\AbstractEntity; use Shlinkio\Shlink\Common\Entity\AbstractEntity;
use Shlinkio\Shlink\Core\Domain\Entity\Domain; use Shlinkio\Shlink\Core\Domain\Entity\Domain;
use Shlinkio\Shlink\Core\Exception\ShortCodeCannotBeRegeneratedException; use Shlinkio\Shlink\Core\Exception\ShortCodeCannotBeRegeneratedException;
use Shlinkio\Shlink\Core\Model\DeviceType;
use Shlinkio\Shlink\Core\ShortUrl\Model\DeviceLongUrlPair; use Shlinkio\Shlink\Core\ShortUrl\Model\DeviceLongUrlPair;
use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlCreation; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlCreation;
use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlEdition; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlEdition;
@@ -170,7 +171,7 @@ class ShortUrl extends AbstractEntity
} }
foreach ($shortUrlEdit->deviceLongUrls as $deviceLongUrlPair) { foreach ($shortUrlEdit->deviceLongUrls as $deviceLongUrlPair) {
$deviceLongUrl = $this->deviceLongUrls->findFirst( $deviceLongUrl = $this->deviceLongUrls->findFirst(
fn ($_, DeviceLongUrl $d) => $d->deviceType() === $deviceLongUrlPair->deviceType, fn ($_, DeviceLongUrl $d) => $d->deviceType === $deviceLongUrlPair->deviceType,
); );
if ($deviceLongUrl !== null) { if ($deviceLongUrl !== null) {
@@ -186,6 +187,15 @@ class ShortUrl extends AbstractEntity
return $this->longUrl; return $this->longUrl;
} }
public function longUrlForDevice(?DeviceType $deviceType): string
{
$deviceLongUrl = $this->deviceLongUrls->findFirst(
static fn ($_, DeviceLongUrl $longUrl) => $longUrl->deviceType === $deviceType,
);
return $deviceLongUrl?->longUrl() ?? $this->longUrl;
}
public function getShortCode(): string public function getShortCode(): string
{ {
return $this->shortCode; return $this->shortCode;
@@ -322,7 +332,7 @@ class ShortUrl extends AbstractEntity
{ {
$data = []; $data = [];
foreach ($this->deviceLongUrls as $deviceUrl) { foreach ($this->deviceLongUrls as $deviceUrl) {
$data[$deviceUrl->deviceType()->value] = $deviceUrl->longUrl(); $data[$deviceUrl->deviceType->value] = $deviceUrl->longUrl();
} }
return $data; return $data;

View File

@@ -8,6 +8,7 @@ use GuzzleHttp\Psr7\Query;
use Laminas\Stdlib\ArrayUtils; use Laminas\Stdlib\ArrayUtils;
use League\Uri\Uri; use League\Uri\Uri;
use Psr\Http\Message\ServerRequestInterface; use Psr\Http\Message\ServerRequestInterface;
use Shlinkio\Shlink\Core\Model\DeviceType;
use Shlinkio\Shlink\Core\Options\TrackingOptions; use Shlinkio\Shlink\Core\Options\TrackingOptions;
use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl;
@@ -25,7 +26,8 @@ class ShortUrlRedirectionBuilder implements ShortUrlRedirectionBuilderInterface
?string $extraPath = null, ?string $extraPath = null,
): string { ): string {
$currentQuery = $request->getQueryParams(); $currentQuery = $request->getQueryParams();
$uri = Uri::createFromString($shortUrl->getLongUrl()); $device = DeviceType::matchFromUserAgent($request->getHeaderLine('User-Agent'));
$uri = Uri::createFromString($shortUrl->longUrlForDevice($device));
$shouldForwardQuery = $shortUrl->forwardQuery(); $shouldForwardQuery = $shortUrl->forwardQuery();
return $uri return $uri

View File

@@ -0,0 +1,38 @@
<?php
declare(strict_types=1);
namespace ShlinkioApiTest\Shlink\Core\Action;
use Shlinkio\Shlink\TestUtils\ApiTest\ApiTestCase;
use const ShlinkioTest\Shlink\ANDROID_USER_AGENT;
use const ShlinkioTest\Shlink\DESKTOP_USER_AGENT;
use const ShlinkioTest\Shlink\IOS_USER_AGENT;
class RedirectTest extends ApiTestCase
{
/**
* @test
* @dataProvider provideUserAgents
*/
public function properRedirectHappensBasedOnUserAgent(?string $userAgent, string $expectedRedirect): void
{
$response = $this->callShortUrl('def456', $userAgent);
self::assertEquals($expectedRedirect, $response->getHeaderLine('Location'));
}
public function provideUserAgents(): iterable
{
yield 'android' => [ANDROID_USER_AGENT, 'https://blog.alejandrocelaya.com/android'];
yield 'ios' => [IOS_USER_AGENT, 'https://blog.alejandrocelaya.com/ios'];
yield 'desktop' => [
DESKTOP_USER_AGENT,
'https://blog.alejandrocelaya.com/2017/12/09/acmailer-7-0-the-most-important-release-in-a-long-time/',
];
yield 'unknown' => [
null,
'https://blog.alejandrocelaya.com/2017/12/09/acmailer-7-0-the-most-important-release-in-a-long-time/',
];
}
}

View File

@@ -6,11 +6,17 @@ namespace ShlinkioTest\Shlink\Core\ShortUrl\Helper;
use Laminas\Diactoros\ServerRequestFactory; use Laminas\Diactoros\ServerRequestFactory;
use PHPUnit\Framework\TestCase; use PHPUnit\Framework\TestCase;
use Psr\Http\Message\ServerRequestInterface;
use Shlinkio\Shlink\Core\Model\DeviceType;
use Shlinkio\Shlink\Core\Options\TrackingOptions; use Shlinkio\Shlink\Core\Options\TrackingOptions;
use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl;
use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlRedirectionBuilder; use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlRedirectionBuilder;
use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlCreation; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlCreation;
use const ShlinkioTest\Shlink\ANDROID_USER_AGENT;
use const ShlinkioTest\Shlink\DESKTOP_USER_AGENT;
use const ShlinkioTest\Shlink\IOS_USER_AGENT;
class ShortUrlRedirectionBuilderTest extends TestCase class ShortUrlRedirectionBuilderTest extends TestCase
{ {
private ShortUrlRedirectionBuilder $redirectionBuilder; private ShortUrlRedirectionBuilder $redirectionBuilder;
@@ -27,78 +33,92 @@ class ShortUrlRedirectionBuilderTest extends TestCase
*/ */
public function buildShortUrlRedirectBuildsExpectedUrl( public function buildShortUrlRedirectBuildsExpectedUrl(
string $expectedUrl, string $expectedUrl,
array $query, ServerRequestInterface $request,
?string $extraPath, ?string $extraPath,
?bool $forwardQuery, ?bool $forwardQuery,
): void { ): void {
$shortUrl = ShortUrl::create(ShortUrlCreation::fromRawData([ $shortUrl = ShortUrl::create(ShortUrlCreation::fromRawData([
'longUrl' => 'https://domain.com/foo/bar?some=thing', 'longUrl' => 'https://domain.com/foo/bar?some=thing',
'forwardQuery' => $forwardQuery, 'forwardQuery' => $forwardQuery,
'deviceLongUrls' => [
DeviceType::ANDROID->value => 'https://domain.com/android',
DeviceType::IOS->value => 'https://domain.com/ios',
],
])); ]));
$result = $this->redirectionBuilder->buildShortUrlRedirect( $result = $this->redirectionBuilder->buildShortUrlRedirect($shortUrl, $request, $extraPath);
$shortUrl,
ServerRequestFactory::fromGlobals()->withQueryParams($query),
$extraPath,
);
self::assertEquals($expectedUrl, $result); self::assertEquals($expectedUrl, $result);
} }
public function provideData(): iterable public function provideData(): iterable
{ {
yield ['https://domain.com/foo/bar?some=thing', [], null, true]; $request = static fn (array $query = []) => ServerRequestFactory::fromGlobals()->withQueryParams($query);
yield ['https://domain.com/foo/bar?some=thing', [], null, null];
yield ['https://domain.com/foo/bar?some=thing', [], null, false]; yield ['https://domain.com/foo/bar?some=thing', $request(), null, true];
yield ['https://domain.com/foo/bar?some=thing&else', ['else' => null], null, true]; yield ['https://domain.com/foo/bar?some=thing', $request(), null, null];
yield ['https://domain.com/foo/bar?some=thing&foo=bar', ['foo' => 'bar'], null, true]; yield ['https://domain.com/foo/bar?some=thing', $request(), null, false];
yield ['https://domain.com/foo/bar?some=thing&foo=bar', ['foo' => 'bar'], null, null]; yield ['https://domain.com/foo/bar?some=thing&else', $request(['else' => null]), null, true];
yield ['https://domain.com/foo/bar?some=thing', ['foo' => 'bar'], null, false]; yield ['https://domain.com/foo/bar?some=thing&foo=bar', $request(['foo' => 'bar']), null, true];
yield ['https://domain.com/foo/bar?some=thing&123=foo', ['123' => 'foo'], null, true]; yield ['https://domain.com/foo/bar?some=thing&foo=bar', $request(['foo' => 'bar']), null, null];
yield ['https://domain.com/foo/bar?some=thing&456=foo', [456 => 'foo'], null, true]; yield ['https://domain.com/foo/bar?some=thing', $request(['foo' => 'bar']), null, false];
yield ['https://domain.com/foo/bar?some=thing&456=foo', [456 => 'foo'], null, null]; yield ['https://domain.com/foo/bar?some=thing&123=foo', $request(['123' => 'foo']), null, true];
yield ['https://domain.com/foo/bar?some=thing', [456 => 'foo'], null, false]; yield ['https://domain.com/foo/bar?some=thing&456=foo', $request([456 => 'foo']), null, true];
yield ['https://domain.com/foo/bar?some=thing&456=foo', $request([456 => 'foo']), null, null];
yield ['https://domain.com/foo/bar?some=thing', $request([456 => 'foo']), null, false];
yield [ yield [
'https://domain.com/foo/bar?some=overwritten&foo=bar', 'https://domain.com/foo/bar?some=overwritten&foo=bar',
['foo' => 'bar', 'some' => 'overwritten'], $request(['foo' => 'bar', 'some' => 'overwritten']),
null, null,
true, true,
]; ];
yield [ yield [
'https://domain.com/foo/bar?some=overwritten', 'https://domain.com/foo/bar?some=overwritten',
['foobar' => 'notrack', 'some' => 'overwritten'], $request(['foobar' => 'notrack', 'some' => 'overwritten'])->withHeader('User-Agent', 'Unknown'),
null, null,
true, true,
]; ];
yield [ yield [
'https://domain.com/foo/bar?some=overwritten', 'https://domain.com/foo/bar?some=overwritten',
['foobar' => 'notrack', 'some' => 'overwritten'], $request(['foobar' => 'notrack', 'some' => 'overwritten']),
null, null,
null, null,
]; ];
yield [ yield [
'https://domain.com/foo/bar?some=thing', 'https://domain.com/foo/bar?some=thing',
['foobar' => 'notrack', 'some' => 'overwritten'], $request(['foobar' => 'notrack', 'some' => 'overwritten']),
null, null,
false, false,
]; ];
yield ['https://domain.com/foo/bar/something/else-baz?some=thing', [], '/something/else-baz', true]; yield ['https://domain.com/foo/bar/something/else-baz?some=thing', $request(), '/something/else-baz', true];
yield [ yield [
'https://domain.com/foo/bar/something/else-baz?some=thing&hello=world', 'https://domain.com/foo/bar/something/else-baz?some=thing&hello=world',
['hello' => 'world'], $request(['hello' => 'world'])->withHeader('User-Agent', DESKTOP_USER_AGENT),
'/something/else-baz', '/something/else-baz',
true, true,
]; ];
yield [ yield [
'https://domain.com/foo/bar/something/else-baz?some=thing&hello=world', 'https://domain.com/foo/bar/something/else-baz?some=thing&hello=world',
['hello' => 'world'], $request(['hello' => 'world']),
'/something/else-baz', '/something/else-baz',
null, null,
]; ];
yield [ yield [
'https://domain.com/foo/bar/something/else-baz?some=thing', 'https://domain.com/foo/bar/something/else-baz?some=thing',
['hello' => 'world'], $request(['hello' => 'world']),
'/something/else-baz', '/something/else-baz',
false, false,
]; ];
yield [
'https://domain.com/android/something',
$request(['foo' => 'bar'])->withHeader('User-Agent', ANDROID_USER_AGENT),
'/something',
false,
];
yield [
'https://domain.com/ios?foo=bar',
$request(['foo' => 'bar'])->withHeader('User-Agent', IOS_USER_AGENT),
null,
null,
];
} }
} }