From b1b67c497ebe997edc34977e09e31d303ccf1d46 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 21 Jan 2023 11:15:38 +0100 Subject: [PATCH] Add logic to dynamically resolve the long URL to redirect to based on requesting device --- composer.json | 5 +- config/test/constants.php | 7 ++ .../src/ShortUrl/Entity/DeviceLongUrl.php | 7 +- module/Core/src/ShortUrl/Entity/ShortUrl.php | 14 +++- .../Helper/ShortUrlRedirectionBuilder.php | 4 +- module/Core/test-api/Action/RedirectTest.php | 38 ++++++++++ .../Helper/ShortUrlRedirectionBuilderTest.php | 70 ++++++++++++------- 7 files changed, 109 insertions(+), 36 deletions(-) create mode 100644 module/Core/test-api/Action/RedirectTest.php diff --git a/composer.json b/composer.json index d7741611..6a279051 100644 --- a/composer.json +++ b/composer.json @@ -74,7 +74,7 @@ "phpunit/phpunit": "^9.5", "roave/security-advisories": "dev-master", "shlinkio/php-coding-standard": "~2.3.0", - "shlinkio/shlink-test-utils": "^3.3", + "shlinkio/shlink-test-utils": "^3.4", "symfony/var-dumper": "^6.1", "veewee/composer-run-parallel": "^1.1" }, @@ -97,7 +97,8 @@ "ShlinkioApiTest\\Shlink\\Rest\\": "module/Rest/test-api", "ShlinkioDbTest\\Shlink\\Rest\\": "module/Rest/test-db", "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": [ "config/test/constants.php" diff --git a/config/test/constants.php b/config/test/constants.php index c767abc9..bce232f3 100644 --- a/config/test/constants.php +++ b/config/test/constants.php @@ -6,3 +6,10 @@ namespace ShlinkioTest\Shlink; const API_TESTS_HOST = '127.0.0.1'; 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'; diff --git a/module/Core/src/ShortUrl/Entity/DeviceLongUrl.php b/module/Core/src/ShortUrl/Entity/DeviceLongUrl.php index 315f7f38..668741e8 100644 --- a/module/Core/src/ShortUrl/Entity/DeviceLongUrl.php +++ b/module/Core/src/ShortUrl/Entity/DeviceLongUrl.php @@ -12,7 +12,7 @@ class DeviceLongUrl extends AbstractEntity { private function __construct( 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, ) { } @@ -27,11 +27,6 @@ class DeviceLongUrl extends AbstractEntity return $this->longUrl; } - public function deviceType(): DeviceType - { - return $this->deviceType; - } - public function updateLongUrl(string $longUrl): void { $this->longUrl = $longUrl; diff --git a/module/Core/src/ShortUrl/Entity/ShortUrl.php b/module/Core/src/ShortUrl/Entity/ShortUrl.php index 3a1b7329..9063bdd7 100644 --- a/module/Core/src/ShortUrl/Entity/ShortUrl.php +++ b/module/Core/src/ShortUrl/Entity/ShortUrl.php @@ -12,6 +12,7 @@ use Doctrine\Common\Collections\Selectable; use Shlinkio\Shlink\Common\Entity\AbstractEntity; use Shlinkio\Shlink\Core\Domain\Entity\Domain; 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\ShortUrlCreation; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlEdition; @@ -170,7 +171,7 @@ class ShortUrl extends AbstractEntity } foreach ($shortUrlEdit->deviceLongUrls as $deviceLongUrlPair) { $deviceLongUrl = $this->deviceLongUrls->findFirst( - fn ($_, DeviceLongUrl $d) => $d->deviceType() === $deviceLongUrlPair->deviceType, + fn ($_, DeviceLongUrl $d) => $d->deviceType === $deviceLongUrlPair->deviceType, ); if ($deviceLongUrl !== null) { @@ -186,6 +187,15 @@ class ShortUrl extends AbstractEntity 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 { return $this->shortCode; @@ -322,7 +332,7 @@ class ShortUrl extends AbstractEntity { $data = []; foreach ($this->deviceLongUrls as $deviceUrl) { - $data[$deviceUrl->deviceType()->value] = $deviceUrl->longUrl(); + $data[$deviceUrl->deviceType->value] = $deviceUrl->longUrl(); } return $data; diff --git a/module/Core/src/ShortUrl/Helper/ShortUrlRedirectionBuilder.php b/module/Core/src/ShortUrl/Helper/ShortUrlRedirectionBuilder.php index 4f457659..c322f195 100644 --- a/module/Core/src/ShortUrl/Helper/ShortUrlRedirectionBuilder.php +++ b/module/Core/src/ShortUrl/Helper/ShortUrlRedirectionBuilder.php @@ -8,6 +8,7 @@ use GuzzleHttp\Psr7\Query; use Laminas\Stdlib\ArrayUtils; use League\Uri\Uri; use Psr\Http\Message\ServerRequestInterface; +use Shlinkio\Shlink\Core\Model\DeviceType; use Shlinkio\Shlink\Core\Options\TrackingOptions; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; @@ -25,7 +26,8 @@ class ShortUrlRedirectionBuilder implements ShortUrlRedirectionBuilderInterface ?string $extraPath = null, ): string { $currentQuery = $request->getQueryParams(); - $uri = Uri::createFromString($shortUrl->getLongUrl()); + $device = DeviceType::matchFromUserAgent($request->getHeaderLine('User-Agent')); + $uri = Uri::createFromString($shortUrl->longUrlForDevice($device)); $shouldForwardQuery = $shortUrl->forwardQuery(); return $uri diff --git a/module/Core/test-api/Action/RedirectTest.php b/module/Core/test-api/Action/RedirectTest.php new file mode 100644 index 00000000..73b6a1cc --- /dev/null +++ b/module/Core/test-api/Action/RedirectTest.php @@ -0,0 +1,38 @@ +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/', + ]; + } +} diff --git a/module/Core/test/ShortUrl/Helper/ShortUrlRedirectionBuilderTest.php b/module/Core/test/ShortUrl/Helper/ShortUrlRedirectionBuilderTest.php index 342ba1ff..341ff6bf 100644 --- a/module/Core/test/ShortUrl/Helper/ShortUrlRedirectionBuilderTest.php +++ b/module/Core/test/ShortUrl/Helper/ShortUrlRedirectionBuilderTest.php @@ -6,11 +6,17 @@ namespace ShlinkioTest\Shlink\Core\ShortUrl\Helper; use Laminas\Diactoros\ServerRequestFactory; 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\ShortUrl\Entity\ShortUrl; use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlRedirectionBuilder; 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 { private ShortUrlRedirectionBuilder $redirectionBuilder; @@ -27,78 +33,92 @@ class ShortUrlRedirectionBuilderTest extends TestCase */ public function buildShortUrlRedirectBuildsExpectedUrl( string $expectedUrl, - array $query, + ServerRequestInterface $request, ?string $extraPath, ?bool $forwardQuery, ): void { $shortUrl = ShortUrl::create(ShortUrlCreation::fromRawData([ 'longUrl' => 'https://domain.com/foo/bar?some=thing', 'forwardQuery' => $forwardQuery, + 'deviceLongUrls' => [ + DeviceType::ANDROID->value => 'https://domain.com/android', + DeviceType::IOS->value => 'https://domain.com/ios', + ], ])); - $result = $this->redirectionBuilder->buildShortUrlRedirect( - $shortUrl, - ServerRequestFactory::fromGlobals()->withQueryParams($query), - $extraPath, - ); + $result = $this->redirectionBuilder->buildShortUrlRedirect($shortUrl, $request, $extraPath); self::assertEquals($expectedUrl, $result); } public function provideData(): iterable { - yield ['https://domain.com/foo/bar?some=thing', [], null, true]; - 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&else', ['else' => null], null, true]; - yield ['https://domain.com/foo/bar?some=thing&foo=bar', ['foo' => 'bar'], null, true]; - yield ['https://domain.com/foo/bar?some=thing&foo=bar', ['foo' => 'bar'], null, null]; - yield ['https://domain.com/foo/bar?some=thing', ['foo' => 'bar'], null, false]; - yield ['https://domain.com/foo/bar?some=thing&123=foo', ['123' => 'foo'], null, true]; - yield ['https://domain.com/foo/bar?some=thing&456=foo', [456 => 'foo'], null, true]; - yield ['https://domain.com/foo/bar?some=thing&456=foo', [456 => 'foo'], null, null]; - yield ['https://domain.com/foo/bar?some=thing', [456 => 'foo'], null, false]; + $request = static fn (array $query = []) => ServerRequestFactory::fromGlobals()->withQueryParams($query); + + yield ['https://domain.com/foo/bar?some=thing', $request(), null, true]; + yield ['https://domain.com/foo/bar?some=thing', $request(), null, null]; + yield ['https://domain.com/foo/bar?some=thing', $request(), null, false]; + yield ['https://domain.com/foo/bar?some=thing&else', $request(['else' => null]), null, true]; + yield ['https://domain.com/foo/bar?some=thing&foo=bar', $request(['foo' => 'bar']), 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', $request(['foo' => 'bar']), null, false]; + 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', $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 [ 'https://domain.com/foo/bar?some=overwritten&foo=bar', - ['foo' => 'bar', 'some' => 'overwritten'], + $request(['foo' => 'bar', 'some' => 'overwritten']), null, true, ]; yield [ 'https://domain.com/foo/bar?some=overwritten', - ['foobar' => 'notrack', 'some' => 'overwritten'], + $request(['foobar' => 'notrack', 'some' => 'overwritten'])->withHeader('User-Agent', 'Unknown'), null, true, ]; yield [ 'https://domain.com/foo/bar?some=overwritten', - ['foobar' => 'notrack', 'some' => 'overwritten'], + $request(['foobar' => 'notrack', 'some' => 'overwritten']), null, null, ]; yield [ 'https://domain.com/foo/bar?some=thing', - ['foobar' => 'notrack', 'some' => 'overwritten'], + $request(['foobar' => 'notrack', 'some' => 'overwritten']), null, 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 [ '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', true, ]; yield [ 'https://domain.com/foo/bar/something/else-baz?some=thing&hello=world', - ['hello' => 'world'], + $request(['hello' => 'world']), '/something/else-baz', null, ]; yield [ 'https://domain.com/foo/bar/something/else-baz?some=thing', - ['hello' => 'world'], + $request(['hello' => 'world']), '/something/else-baz', 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, + ]; } }