diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a95f8f21..2ccb5ae7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -60,6 +60,8 @@ jobs: strategy: matrix: php-version: ['8.1', '8.2'] + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} # rr get-binary picks this env automatically steps: - uses: actions/checkout@v3 - run: docker-compose -f docker-compose.yml -f docker-compose.ci.yml up -d shlink_db_postgres diff --git a/CHANGELOG.md b/CHANGELOG.md index 55ce701d..58031200 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,14 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this ## [Unreleased] ### Added +* [#1557](https://github.com/shlinkio/shlink/issues/1557) Added support to dynamically redirect to different long URLs based on the visitor's device type. + + For the moment, only `android`, `ios` and `desktop` can have their own specific long URL, and when the visitor cannot be matched against any of them, the regular long URL will be used. + + In the future, more granular device types could be added if appropriate (iOS tablet, android table, tablet, mobile phone, Linux, Mac, Windows, etc). + + In order to match the visitor's device, the `User-Agent` header is used. + * [#1632](https://github.com/shlinkio/shlink/issues/1632) Added amount of bots, non-bots and total visits to the visits summary endpoint. * [#1633](https://github.com/shlinkio/shlink/issues/1633) Added amount of bots, non-bots and total visits to the tag stats endpoint. * [#1653](https://github.com/shlinkio/shlink/issues/1653) Added support for all HTTP methods in short URLs, together with two new redirect status codes, 307 and 308. @@ -20,7 +28,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this * *Nothing* ### Deprecated -* *Nothing* +* [#1676](https://github.com/shlinkio/shlink/issues/1676) Deprecated `GET /short-urls/shorten` endpoint. Use `POST /short-urls` to create short URLs instead ### Removed * *Nothing* diff --git a/composer.json b/composer.json index 85608742..6a279051 100644 --- a/composer.json +++ b/composer.json @@ -40,12 +40,13 @@ "mezzio/mezzio-problem-details": "^1.7", "mezzio/mezzio-swoole": "^4.5", "mlocati/ip-lib": "^1.18", + "mobiledetect/mobiledetectlib": "^3.74", "ocramius/proxy-manager": "^2.14", "pagerfanta/core": "^3.6", "php-middleware/request-id": "^4.1", "pugx/shortid-php": "^1.1", "ramsey/uuid": "^4.5", - "shlinkio/shlink-common": "^5.2", + "shlinkio/shlink-common": "dev-main#61d26e7 as 5.3", "shlinkio/shlink-config": "dev-main#2a5b5c2 as 2.4", "shlinkio/shlink-event-dispatcher": "^2.6", "shlinkio/shlink-importer": "^5.0", @@ -73,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" }, @@ -96,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/config.php b/config/config.php index 8fe311a0..e0ec6c23 100644 --- a/config/config.php +++ b/config/config.php @@ -15,6 +15,7 @@ use function class_exists; use function Shlinkio\Shlink\Config\env; use function Shlinkio\Shlink\Config\openswooleIsInstalled; use function Shlinkio\Shlink\Config\runningInRoadRunner; +use function Shlinkio\Shlink\Core\enumValues; use const PHP_SAPI; @@ -23,7 +24,7 @@ $enableSwoole = PHP_SAPI === 'cli' && openswooleIsInstalled() && ! runningInRoad return (new ConfigAggregator\ConfigAggregator([ ! $isTestEnv - ? new EnvVarLoaderProvider('config/params/generated_config.php', Core\Config\EnvVars::values()) + ? new EnvVarLoaderProvider('config/params/generated_config.php', enumValues(Core\Config\EnvVars::class)) : new ConfigAggregator\ArrayProvider([]), Mezzio\ConfigProvider::class, Mezzio\Router\ConfigProvider::class, 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/data/migrations/Version20230103105343.php b/data/migrations/Version20230103105343.php new file mode 100644 index 00000000..c61a8a94 --- /dev/null +++ b/data/migrations/Version20230103105343.php @@ -0,0 +1,53 @@ +skipIf($schema->hasTable(self::TABLE_NAME)); + + $table = $schema->createTable(self::TABLE_NAME); + $table->addColumn('id', Types::BIGINT, [ + 'unsigned' => true, + 'autoincrement' => true, + 'notnull' => true, + ]); + $table->setPrimaryKey(['id']); + + $table->addColumn('device_type', Types::STRING, ['length' => 255]); + $table->addColumn('long_url', Types::STRING, ['length' => 2048]); + $table->addColumn('short_url_id', Types::BIGINT, [ + 'unsigned' => true, + 'notnull' => true, + ]); + + $table->addForeignKeyConstraint('short_urls', ['short_url_id'], ['id'], [ + 'onDelete' => 'CASCADE', + 'onUpdate' => 'RESTRICT', + ]); + + $table->addUniqueIndex(['device_type', 'short_url_id'], 'UQ_device_type_per_short_url'); + } + + public function down(Schema $schema): void + { + $this->skipIf(! $schema->hasTable(self::TABLE_NAME)); + $schema->dropTable(self::TABLE_NAME); + } + + public function isTransactional(): bool + { + return ! ($this->connection->getDatabasePlatform() instanceof MySQLPlatform); + } +} diff --git a/docs/async-api/async-api.json b/docs/async-api/async-api.json index 418409cf..d45dae2b 100644 --- a/docs/async-api/async-api.json +++ b/docs/async-api/async-api.json @@ -111,6 +111,9 @@ "type": "string", "description": "The original long URL." }, + "deviceLongUrls": { + "$ref": "#/components/schemas/DeviceLongUrls" + }, "dateCreated": { "type": "string", "format": "date-time", @@ -152,6 +155,11 @@ "shortCode": "12C18", "shortUrl": "https://s.test/12C18", "longUrl": "https://store.steampowered.com", + "deviceLongUrls": { + "android": "https://store.steampowered.com/android", + "ios": "https://store.steampowered.com/ios", + "desktop": null + }, "dateCreated": "2016-08-21T20:34:16+02:00", "visitsSummary": { "total": 328, @@ -215,6 +223,24 @@ } } }, + "DeviceLongUrls": { + "type": "object", + "required": ["android", "ios", "desktop"], + "properties": { + "android": { + "description": "The long URL to redirect to when the short URL is visited from a device running Android", + "type": "string" + }, + "ios": { + "description": "The long URL to redirect to when the short URL is visited from a device running iOS", + "type": "string" + }, + "desktop": { + "description": "The long URL to redirect to when the short URL is visited from a desktop browser", + "type": "string" + } + } + }, "Visit": { "type": "object", "properties": { diff --git a/docs/swagger/definitions/DeviceLongUrls.json b/docs/swagger/definitions/DeviceLongUrls.json new file mode 100644 index 00000000..1a56d9ef --- /dev/null +++ b/docs/swagger/definitions/DeviceLongUrls.json @@ -0,0 +1,20 @@ +{ + "type": "object", + "properties": { + "android": { + "description": "The long URL to redirect to when the short URL is visited from a device running Android", + "type": "string", + "nullable": false + }, + "ios": { + "description": "The long URL to redirect to when the short URL is visited from a device running iOS", + "type": "string", + "nullable": false + }, + "desktop": { + "description": "The long URL to redirect to when the short URL is visited from a desktop browser", + "type": "string", + "nullable": false + } + } +} diff --git a/docs/swagger/definitions/DeviceLongUrlsEdit.json b/docs/swagger/definitions/DeviceLongUrlsEdit.json new file mode 100644 index 00000000..78f77e46 --- /dev/null +++ b/docs/swagger/definitions/DeviceLongUrlsEdit.json @@ -0,0 +1,17 @@ +{ + "type": "object", + "allOf": [{ + "$ref": "./DeviceLongUrls.json" + }], + "properties": { + "android": { + "nullable": true + }, + "ios": { + "nullable": true + }, + "desktop": { + "nullable": true + } + } +} diff --git a/docs/swagger/definitions/DeviceLongUrlsResp.json b/docs/swagger/definitions/DeviceLongUrlsResp.json new file mode 100644 index 00000000..95724581 --- /dev/null +++ b/docs/swagger/definitions/DeviceLongUrlsResp.json @@ -0,0 +1,7 @@ +{ + "type": "object", + "required": ["android", "ios", "desktop"], + "allOf": [{ + "$ref": "./DeviceLongUrlsEdit.json" + }] +} diff --git a/docs/swagger/definitions/ShortUrl.json b/docs/swagger/definitions/ShortUrl.json index 4d5d9f2d..4060e2f2 100644 --- a/docs/swagger/definitions/ShortUrl.json +++ b/docs/swagger/definitions/ShortUrl.json @@ -4,6 +4,7 @@ "shortCode", "shortUrl", "longUrl", + "deviceLongUrls", "dateCreated", "visitsCount", "visitsSummary", @@ -27,6 +28,9 @@ "type": "string", "description": "The original long URL." }, + "deviceLongUrls": { + "$ref": "./DeviceLongUrlsResp.json" + }, "dateCreated": { "type": "string", "format": "date-time", diff --git a/docs/swagger/definitions/ShortUrlEdition.json b/docs/swagger/definitions/ShortUrlEdition.json index 94ef6135..28fa71bc 100644 --- a/docs/swagger/definitions/ShortUrlEdition.json +++ b/docs/swagger/definitions/ShortUrlEdition.json @@ -5,6 +5,9 @@ "description": "The long URL this short URL will redirect to", "type": "string" }, + "deviceLongUrls": { + "$ref": "./DeviceLongUrlsEdit.json" + }, "validSince": { "description": "The date (in ISO-8601 format) from which this short code will be valid", "type": "string", diff --git a/docs/swagger/paths/v1_short-urls.json b/docs/swagger/paths/v1_short-urls.json index 2bd461d8..76d87659 100644 --- a/docs/swagger/paths/v1_short-urls.json +++ b/docs/swagger/paths/v1_short-urls.json @@ -163,6 +163,11 @@ "shortCode": "12C18", "shortUrl": "https://s.test/12C18", "longUrl": "https://store.steampowered.com", + "deviceLongUrls": { + "android": null, + "ios": null, + "desktop": null + }, "dateCreated": "2016-08-21T20:34:16+02:00", "visitsSummary": { "total": 328, @@ -186,6 +191,11 @@ "shortCode": "12Kb3", "shortUrl": "https://s.test/12Kb3", "longUrl": "https://shlink.io", + "deviceLongUrls": { + "android": null, + "ios": "https://shlink.io/ios", + "desktop": null + }, "dateCreated": "2016-05-01T20:34:16+02:00", "visitsSummary": { "total": 1029, @@ -208,6 +218,11 @@ "shortCode": "123bA", "shortUrl": "https://example.com/123bA", "longUrl": "https://www.google.com", + "deviceLongUrls": { + "android": null, + "ios": null, + "desktop": null + }, "dateCreated": "2015-10-01T20:34:16+02:00", "visitsSummary": { "total": 25, @@ -281,6 +296,9 @@ "type": "object", "required": ["longUrl"], "properties": { + "deviceLongUrls": { + "$ref": "../definitions/DeviceLongUrls.json" + }, "customSlug": { "description": "A unique custom slug to be used instead of the generated short code", "type": "string" @@ -320,6 +338,11 @@ "shortCode": "12C18", "shortUrl": "https://s.test/12C18", "longUrl": "https://store.steampowered.com", + "deviceLongUrls": { + "android": null, + "ios": null, + "desktop": null + }, "dateCreated": "2016-08-21T20:34:16+02:00", "visitsSummary": { "total": 0, diff --git a/docs/swagger/paths/v1_short-urls_shorten.json b/docs/swagger/paths/v1_short-urls_shorten.json index e0257c59..cacb00bb 100644 --- a/docs/swagger/paths/v1_short-urls_shorten.json +++ b/docs/swagger/paths/v1_short-urls_shorten.json @@ -1,11 +1,12 @@ { "get": { "operationId": "shortenUrl", + "deprecated": true, "tags": [ "Short URLs" ], "summary": "Create a short URL", - "description": "Creates a short URL in a single API call. Useful for third party integrations.", + "description": "**[Deprecated]** Use [Create short URL](#/Short%20URLs/createShortUrl) instead", "parameters": [ { "$ref": "../parameters/version.json" @@ -52,6 +53,11 @@ }, "example": { "longUrl": "https://github.com/shlinkio/shlink", + "deviceLongUrls": { + "android": null, + "ios": null, + "desktop": null + }, "shortUrl": "https://s.test/abc123", "shortCode": "abc123", "dateCreated": "2016-08-21T20:34:16+02:00", diff --git a/docs/swagger/paths/v1_short-urls_{shortCode}.json b/docs/swagger/paths/v1_short-urls_{shortCode}.json index bd69b4ab..e639f362 100644 --- a/docs/swagger/paths/v1_short-urls_{shortCode}.json +++ b/docs/swagger/paths/v1_short-urls_{shortCode}.json @@ -40,6 +40,11 @@ "shortCode": "12Kb3", "shortUrl": "https://s.test/12Kb3", "longUrl": "https://shlink.io", + "deviceLongUrls": { + "android": null, + "ios": null, + "desktop": null + }, "dateCreated": "2016-05-01T20:34:16+02:00", "visitsSummary": { "total": 1029, @@ -162,6 +167,11 @@ "shortCode": "12Kb3", "shortUrl": "https://s.test/12Kb3", "longUrl": "https://shlink.io", + "deviceLongUrls": { + "android": "https://shlink.io/android", + "ios": null, + "desktop": null + }, "dateCreated": "2016-05-01T20:34:16+02:00", "visitsSummary": { "total": 1029, diff --git a/module/CLI/test/Command/Domain/GetDomainVisitsCommandTest.php b/module/CLI/test/Command/Domain/GetDomainVisitsCommandTest.php index 913e00dc..5cda6dc3 100644 --- a/module/CLI/test/Command/Domain/GetDomainVisitsCommandTest.php +++ b/module/CLI/test/Command/Domain/GetDomainVisitsCommandTest.php @@ -40,7 +40,7 @@ class GetDomainVisitsCommandTest extends TestCase /** @test */ public function outputIsProperlyGenerated(): void { - $shortUrl = ShortUrl::createEmpty(); + $shortUrl = ShortUrl::createFake(); $visit = Visit::forValidShortUrl($shortUrl, new Visitor('bar', 'foo', '', ''))->locate( VisitLocation::fromGeolocation(new Location('', 'Spain', '', 'Madrid', 0, 0, '')), ); diff --git a/module/CLI/test/Command/ShortUrl/CreateShortUrlCommandTest.php b/module/CLI/test/Command/ShortUrl/CreateShortUrlCommandTest.php index 734089c9..69ce0c72 100644 --- a/module/CLI/test/Command/ShortUrl/CreateShortUrlCommandTest.php +++ b/module/CLI/test/Command/ShortUrl/CreateShortUrlCommandTest.php @@ -48,7 +48,7 @@ class CreateShortUrlCommandTest extends TestCase /** @test */ public function properShortCodeIsCreatedIfLongUrlIsCorrect(): void { - $shortUrl = ShortUrl::createEmpty(); + $shortUrl = ShortUrl::createFake(); $this->urlShortener->expects($this->once())->method('shorten')->withAnyParameters()->willReturn($shortUrl); $this->stringifier->expects($this->once())->method('stringify')->with($shortUrl)->willReturn( 'stringified_short_url', @@ -98,11 +98,10 @@ class CreateShortUrlCommandTest extends TestCase /** @test */ public function properlyProcessesProvidedTags(): void { - $shortUrl = ShortUrl::createEmpty(); + $shortUrl = ShortUrl::createFake(); $this->urlShortener->expects($this->once())->method('shorten')->with( - $this->callback(function (ShortUrlCreation $meta) { - $tags = $meta->getTags(); - Assert::assertEquals(['foo', 'bar', 'baz', 'boo', 'zar'], $tags); + $this->callback(function (ShortUrlCreation $creation) { + Assert::assertEquals(['foo', 'bar', 'baz', 'boo', 'zar'], $creation->tags); return true; }), )->willReturn($shortUrl); @@ -128,10 +127,10 @@ class CreateShortUrlCommandTest extends TestCase { $this->urlShortener->expects($this->once())->method('shorten')->with( $this->callback(function (ShortUrlCreation $meta) use ($expectedDomain) { - Assert::assertEquals($expectedDomain, $meta->getDomain()); + Assert::assertEquals($expectedDomain, $meta->domain); return true; }), - )->willReturn(ShortUrl::createEmpty()); + )->willReturn(ShortUrl::createFake()); $this->stringifier->method('stringify')->with($this->isInstanceOf(ShortUrl::class))->willReturn(''); $input['longUrl'] = 'http://domain.com/foo/bar'; @@ -154,7 +153,7 @@ class CreateShortUrlCommandTest extends TestCase */ public function urlValidationHasExpectedValueBasedOnProvidedFlags(array $options, ?bool $expectedValidateUrl): void { - $shortUrl = ShortUrl::createEmpty(); + $shortUrl = ShortUrl::createFake(); $this->urlShortener->expects($this->once())->method('shorten')->with( $this->callback(function (ShortUrlCreation $meta) use ($expectedValidateUrl) { Assert::assertEquals($expectedValidateUrl, $meta->doValidateUrl()); diff --git a/module/CLI/test/Command/ShortUrl/GetShortUrlVisitsCommandTest.php b/module/CLI/test/Command/ShortUrl/GetShortUrlVisitsCommandTest.php index 8706699b..bd2be187 100644 --- a/module/CLI/test/Command/ShortUrl/GetShortUrlVisitsCommandTest.php +++ b/module/CLI/test/Command/ShortUrl/GetShortUrlVisitsCommandTest.php @@ -94,7 +94,7 @@ class GetShortUrlVisitsCommandTest extends TestCase /** @test */ public function outputIsProperlyGenerated(): void { - $visit = Visit::forValidShortUrl(ShortUrl::createEmpty(), new Visitor('bar', 'foo', '', ''))->locate( + $visit = Visit::forValidShortUrl(ShortUrl::createFake(), new Visitor('bar', 'foo', '', ''))->locate( VisitLocation::fromGeolocation(new Location('', 'Spain', '', 'Madrid', 0, 0, '')), ); $shortCode = 'abc123'; diff --git a/module/CLI/test/Command/Tag/GetTagVisitsCommandTest.php b/module/CLI/test/Command/Tag/GetTagVisitsCommandTest.php index be56cdee..b7255d0a 100644 --- a/module/CLI/test/Command/Tag/GetTagVisitsCommandTest.php +++ b/module/CLI/test/Command/Tag/GetTagVisitsCommandTest.php @@ -40,7 +40,7 @@ class GetTagVisitsCommandTest extends TestCase /** @test */ public function outputIsProperlyGenerated(): void { - $shortUrl = ShortUrl::createEmpty(); + $shortUrl = ShortUrl::createFake(); $visit = Visit::forValidShortUrl($shortUrl, new Visitor('bar', 'foo', '', ''))->locate( VisitLocation::fromGeolocation(new Location('', 'Spain', '', 'Madrid', 0, 0, '')), ); diff --git a/module/CLI/test/Command/Visit/GetNonOrphanVisitsCommandTest.php b/module/CLI/test/Command/Visit/GetNonOrphanVisitsCommandTest.php index 90147541..c780208a 100644 --- a/module/CLI/test/Command/Visit/GetNonOrphanVisitsCommandTest.php +++ b/module/CLI/test/Command/Visit/GetNonOrphanVisitsCommandTest.php @@ -40,7 +40,7 @@ class GetNonOrphanVisitsCommandTest extends TestCase /** @test */ public function outputIsProperlyGenerated(): void { - $shortUrl = ShortUrl::createEmpty(); + $shortUrl = ShortUrl::createFake(); $visit = Visit::forValidShortUrl($shortUrl, new Visitor('bar', 'foo', '', ''))->locate( VisitLocation::fromGeolocation(new Location('', 'Spain', '', 'Madrid', 0, 0, '')), ); diff --git a/module/CLI/test/Command/Visit/LocateVisitsCommandTest.php b/module/CLI/test/Command/Visit/LocateVisitsCommandTest.php index 518d9f45..44638249 100644 --- a/module/CLI/test/Command/Visit/LocateVisitsCommandTest.php +++ b/module/CLI/test/Command/Visit/LocateVisitsCommandTest.php @@ -66,7 +66,7 @@ class LocateVisitsCommandTest extends TestCase bool $expectWarningPrint, array $args, ): void { - $visit = Visit::forValidShortUrl(ShortUrl::createEmpty(), new Visitor('', '', '1.2.3.4', '')); + $visit = Visit::forValidShortUrl(ShortUrl::createFake(), new Visitor('', '', '1.2.3.4', '')); $location = VisitLocation::fromGeolocation(Location::emptyInstance()); $mockMethodBehavior = $this->invokeHelperMethods($visit, $location); @@ -113,7 +113,7 @@ class LocateVisitsCommandTest extends TestCase */ public function localhostAndEmptyAddressesAreIgnored(IpCannotBeLocatedException $e, string $message): void { - $visit = Visit::forValidShortUrl(ShortUrl::createEmpty(), Visitor::emptyInstance()); + $visit = Visit::forValidShortUrl(ShortUrl::createFake(), Visitor::emptyInstance()); $location = VisitLocation::fromGeolocation(Location::emptyInstance()); $this->lock->method('acquire')->with($this->isFalse())->willReturn(true); @@ -140,7 +140,7 @@ class LocateVisitsCommandTest extends TestCase /** @test */ public function errorWhileLocatingIpIsDisplayed(): void { - $visit = Visit::forValidShortUrl(ShortUrl::createEmpty(), new Visitor('', '', '1.2.3.4', '')); + $visit = Visit::forValidShortUrl(ShortUrl::createFake(), new Visitor('', '', '1.2.3.4', '')); $location = VisitLocation::fromGeolocation(Location::emptyInstance()); $this->lock->method('acquire')->with($this->isFalse())->willReturn(true); diff --git a/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.ShortUrl.Entity.DeviceLongUrl.php b/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.ShortUrl.Entity.DeviceLongUrl.php new file mode 100644 index 00000000..8de69c18 --- /dev/null +++ b/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.ShortUrl.Entity.DeviceLongUrl.php @@ -0,0 +1,41 @@ +setTable(determineTableName('device_long_urls', $emConfig)); + + $builder->createField('id', Types::BIGINT) + ->columnName('id') + ->makePrimaryKey() + ->generatedValue('IDENTITY') + ->option('unsigned', true) + ->build(); + + (new FieldBuilder($builder, [ + 'fieldName' => 'deviceType', + 'type' => Types::STRING, + 'enumType' => DeviceType::class, + ]))->columnName('device_type') + ->length(255) + ->build(); + + fieldWithUtf8Charset($builder->createField('longUrl', Types::STRING), $emConfig) + ->columnName('long_url') + ->length(2048) + ->build(); + + $builder->createManyToOne('shortUrl', ShortUrl\Entity\ShortUrl::class) + ->addJoinColumn('short_url_id', 'id', false, false, 'CASCADE') + ->build(); +}; diff --git a/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.ShortUrl.Entity.ShortUrl.php b/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.ShortUrl.Entity.ShortUrl.php index 6b769f34..746ac3fd 100644 --- a/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.ShortUrl.Entity.ShortUrl.php +++ b/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.ShortUrl.Entity.ShortUrl.php @@ -24,7 +24,7 @@ return static function (ClassMetadata $metadata, array $emConfig): void { ->build(); fieldWithUtf8Charset($builder->createField('longUrl', Types::STRING), $emConfig) - ->columnName('original_url') + ->columnName('original_url') // Rename to long_url some day? ¯\_(ツ)_/¯ ->length(2048) ->build(); @@ -67,6 +67,13 @@ return static function (ClassMetadata $metadata, array $emConfig): void { ->fetchExtraLazy() ->build(); + $builder->createOneToMany('deviceLongUrls', ShortUrl\Entity\DeviceLongUrl::class) + ->mappedBy('shortUrl') + ->cascadePersist() + ->orphanRemoval() + ->setIndexBy('deviceType') + ->build(); + $builder->createManyToMany('tags', Tag\Entity\Tag::class) ->setJoinTable(determineTableName('short_urls_in_tags', $emConfig)) ->addInverseJoinColumn('tag_id', 'id', true, false, 'CASCADE') diff --git a/module/Core/functions/functions.php b/module/Core/functions/functions.php index 9d0b8d68..574d604c 100644 --- a/module/Core/functions/functions.php +++ b/module/Core/functions/functions.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core; +use BackedEnum; use Cake\Chronos\Chronos; use Cake\Chronos\ChronosInterface; use DateTimeInterface; @@ -16,6 +17,7 @@ use PUGX\Shortid\Factory as ShortIdFactory; use Shlinkio\Shlink\Common\Util\DateRange; use function date_default_timezone_get; +use function Functional\map; use function Functional\reduce_left; use function is_array; use function print_r; @@ -159,3 +161,19 @@ function toProblemDetailsType(string $errorCode): string { return sprintf('https://shlink.io/api/error/%s', $errorCode); } + +/** + * @param class-string $enum + * @return string[] + */ +function enumValues(string $enum): array +{ + static $cache; + if ($cache === null) { + $cache = []; + } + + return $cache[$enum] ?? ( + $cache[$enum] = map($enum::cases(), static fn (BackedEnum $type) => (string) $type->value) + ); +} diff --git a/module/Core/src/Action/RedirectAction.php b/module/Core/src/Action/RedirectAction.php index 725e402d..942cf550 100644 --- a/module/Core/src/Action/RedirectAction.php +++ b/module/Core/src/Action/RedirectAction.php @@ -18,15 +18,15 @@ class RedirectAction extends AbstractTrackingAction implements StatusCodeInterfa public function __construct( ShortUrlResolverInterface $urlResolver, RequestTrackerInterface $requestTracker, - private ShortUrlRedirectionBuilderInterface $redirectionBuilder, - private RedirectResponseHelperInterface $redirectResponseHelper, + private readonly ShortUrlRedirectionBuilderInterface $redirectionBuilder, + private readonly RedirectResponseHelperInterface $redirectResponseHelper, ) { parent::__construct($urlResolver, $requestTracker); } protected function createSuccessResp(ShortUrl $shortUrl, ServerRequestInterface $request): Response { - $longUrl = $this->redirectionBuilder->buildShortUrlRedirect($shortUrl, $request->getQueryParams()); + $longUrl = $this->redirectionBuilder->buildShortUrlRedirect($shortUrl, $request); return $this->redirectResponseHelper->buildRedirectResponse($longUrl); } } diff --git a/module/Core/src/Config/EnvVars.php b/module/Core/src/Config/EnvVars.php index 228a5921..75454ecc 100644 --- a/module/Core/src/Config/EnvVars.php +++ b/module/Core/src/Config/EnvVars.php @@ -4,7 +4,6 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Config; -use function Functional\map; use function Shlinkio\Shlink\Config\env; enum EnvVars: string @@ -77,13 +76,4 @@ enum EnvVars: string { return $this->loadFromEnv() !== null; } - - /** - * @return string[] - */ - public static function values(): array - { - static $values; - return $values ?? ($values = map(self::cases(), static fn (EnvVars $envVar) => $envVar->value)); - } } diff --git a/module/Core/src/Domain/DomainService.php b/module/Core/src/Domain/DomainService.php index 3ecc9f03..703f77fd 100644 --- a/module/Core/src/Domain/DomainService.php +++ b/module/Core/src/Domain/DomainService.php @@ -51,7 +51,7 @@ class DomainService implements DomainServiceInterface $repo = $this->em->getRepository(Domain::class); $groups = group( $repo->findDomains($apiKey), - fn (Domain $domain) => $domain->getAuthority() === $this->defaultDomain ? 'default' : 'domains', + fn (Domain $domain) => $domain->authority === $this->defaultDomain ? 'default' : 'domains', ); return [first($groups['default'] ?? []), $groups['domains'] ?? []]; diff --git a/module/Core/src/Domain/Entity/Domain.php b/module/Core/src/Domain/Entity/Domain.php index ab33ae17..4e6ea865 100644 --- a/module/Core/src/Domain/Entity/Domain.php +++ b/module/Core/src/Domain/Entity/Domain.php @@ -15,7 +15,7 @@ class Domain extends AbstractEntity implements JsonSerializable, NotFoundRedirec private ?string $regular404Redirect = null; private ?string $invalidShortUrlRedirect = null; - private function __construct(private string $authority) + private function __construct(public readonly string $authority) { } @@ -24,14 +24,9 @@ class Domain extends AbstractEntity implements JsonSerializable, NotFoundRedirec return new self($authority); } - public function getAuthority(): string - { - return $this->authority; - } - public function jsonSerialize(): string { - return $this->getAuthority(); + return $this->authority; } public function invalidShortUrlRedirect(): ?string diff --git a/module/Core/src/Domain/Model/DomainItem.php b/module/Core/src/Domain/Model/DomainItem.php index 72ea3e1f..53f2b6f7 100644 --- a/module/Core/src/Domain/Model/DomainItem.php +++ b/module/Core/src/Domain/Model/DomainItem.php @@ -20,7 +20,7 @@ final class DomainItem implements JsonSerializable public static function forNonDefaultDomain(Domain $domain): self { - return new self($domain->getAuthority(), $domain, false); + return new self($domain->authority, $domain, false); } public static function forDefaultDomain(string $defaultDomain, NotFoundRedirectConfigInterface $config): self diff --git a/module/Core/src/Model/DeviceType.php b/module/Core/src/Model/DeviceType.php new file mode 100644 index 00000000..df4a1838 --- /dev/null +++ b/module/Core/src/Model/DeviceType.php @@ -0,0 +1,28 @@ +is('iOS') && $detect->isTablet() => self::IOS, // TODO To detect iPad only +// $detect->is('iOS') && ! $detect->isTablet() => self::IOS, // TODO To detect iPhone only +// $detect->is('androidOS') && $detect->isTablet() => self::ANDROID, // TODO To detect Android tablets +// $detect->is('androidOS') && ! $detect->isTablet() => self::ANDROID, // TODO To detect Android phones + $detect->is('iOS') => self::IOS, // Detects both iPhone and iPad + $detect->is('androidOS') => self::ANDROID, // Detects both android phones and android tablets + ! $detect->isMobile() && ! $detect->isTablet() => self::DESKTOP, + default => null, + }; + } +} diff --git a/module/Core/src/ShortUrl/Entity/DeviceLongUrl.php b/module/Core/src/ShortUrl/Entity/DeviceLongUrl.php new file mode 100644 index 00000000..668741e8 --- /dev/null +++ b/module/Core/src/ShortUrl/Entity/DeviceLongUrl.php @@ -0,0 +1,34 @@ +deviceType, $pair->longUrl); + } + + public function longUrl(): string + { + return $this->longUrl; + } + + 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 0ebdeb24..d0e9cba4 100644 --- a/module/Core/src/ShortUrl/Entity/ShortUrl.php +++ b/module/Core/src/ShortUrl/Entity/ShortUrl.php @@ -12,6 +12,8 @@ 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; use Shlinkio\Shlink\Core\ShortUrl\Model\Validation\ShortUrlInputFilter; @@ -23,7 +25,10 @@ use Shlinkio\Shlink\Core\Visit\Model\VisitType; use Shlinkio\Shlink\Importer\Model\ImportedShlinkUrl; use Shlinkio\Shlink\Rest\Entity\ApiKey; +use function array_fill_keys; use function count; +use function Functional\map; +use function Shlinkio\Shlink\Core\enumValues; use function Shlinkio\Shlink\Core\generateRandomShortCode; use function Shlinkio\Shlink\Core\normalizeDate; use function Shlinkio\Shlink\Core\normalizeOptionalDate; @@ -35,6 +40,8 @@ class ShortUrl extends AbstractEntity private Chronos $dateCreated; /** @var Collection */ private Collection $visits; + /** @var Collection */ + private Collection $deviceLongUrls; /** @var Collection */ private Collection $tags; private ?Chronos $validSince = null; @@ -55,11 +62,14 @@ class ShortUrl extends AbstractEntity { } - public static function createEmpty(): self + public static function createFake(): self { - return self::create(ShortUrlCreation::createEmpty()); + return self::withLongUrl('foo'); } + /** + * @param non-empty-string $longUrl + */ public static function withLongUrl(string $longUrl): self { return self::create(ShortUrlCreation::fromRawData([ShortUrlInputFilter::LONG_URL => $longUrl])); @@ -75,19 +85,23 @@ class ShortUrl extends AbstractEntity $instance->longUrl = $creation->getLongUrl(); $instance->dateCreated = Chronos::now(); $instance->visits = new ArrayCollection(); - $instance->tags = $relationResolver->resolveTags($creation->getTags()); - $instance->validSince = $creation->getValidSince(); - $instance->validUntil = $creation->getValidUntil(); - $instance->maxVisits = $creation->getMaxVisits(); + $instance->deviceLongUrls = new ArrayCollection(map( + $creation->deviceLongUrls, + fn (DeviceLongUrlPair $pair) => DeviceLongUrl::fromShortUrlAndPair($instance, $pair), + )); + $instance->tags = $relationResolver->resolveTags($creation->tags); + $instance->validSince = $creation->validSince; + $instance->validUntil = $creation->validUntil; + $instance->maxVisits = $creation->maxVisits; $instance->customSlugWasProvided = $creation->hasCustomSlug(); - $instance->shortCodeLength = $creation->getShortCodeLength(); - $instance->shortCode = $creation->getCustomSlug() ?? generateRandomShortCode($instance->shortCodeLength); - $instance->domain = $relationResolver->resolveDomain($creation->getDomain()); - $instance->authorApiKey = $creation->getApiKey(); - $instance->title = $creation->getTitle(); - $instance->titleWasAutoResolved = $creation->titleWasAutoResolved(); - $instance->crawlable = $creation->isCrawlable(); - $instance->forwardQuery = $creation->forwardQuery(); + $instance->shortCodeLength = $creation->shortCodeLength; + $instance->shortCode = $creation->customSlug ?? generateRandomShortCode($instance->shortCodeLength); + $instance->domain = $relationResolver->resolveDomain($creation->domain); + $instance->authorApiKey = $creation->apiKey; + $instance->title = $creation->title; + $instance->titleWasAutoResolved = $creation->titleWasAutoResolved; + $instance->crawlable = $creation->crawlable; + $instance->forwardQuery = $creation->forwardQuery; return $instance; } @@ -120,11 +134,68 @@ class ShortUrl extends AbstractEntity return $instance; } + public function update( + ShortUrlEdition $shortUrlEdit, + ?ShortUrlRelationResolverInterface $relationResolver = null, + ): void { + if ($shortUrlEdit->validSinceWasProvided()) { + $this->validSince = $shortUrlEdit->validSince; + } + if ($shortUrlEdit->validUntilWasProvided()) { + $this->validUntil = $shortUrlEdit->validUntil; + } + if ($shortUrlEdit->maxVisitsWasProvided()) { + $this->maxVisits = $shortUrlEdit->maxVisits; + } + if ($shortUrlEdit->longUrlWasProvided()) { + $this->longUrl = $shortUrlEdit->longUrl ?? $this->longUrl; + } + if ($shortUrlEdit->tagsWereProvided()) { + $relationResolver = $relationResolver ?? new SimpleShortUrlRelationResolver(); + $this->tags = $relationResolver->resolveTags($shortUrlEdit->tags); + } + if ($shortUrlEdit->crawlableWasProvided()) { + $this->crawlable = $shortUrlEdit->crawlable; + } + if ( + $this->title === null + || $shortUrlEdit->titleWasProvided() + || ($this->titleWasAutoResolved && $shortUrlEdit->titleWasAutoResolved()) + ) { + $this->title = $shortUrlEdit->title; + $this->titleWasAutoResolved = $shortUrlEdit->titleWasAutoResolved(); + } + if ($shortUrlEdit->forwardQueryWasProvided()) { + $this->forwardQuery = $shortUrlEdit->forwardQuery; + } + + // Update device long URLs, removing, editing or creating where appropriate + foreach ($shortUrlEdit->devicesToRemove as $deviceType) { + $this->deviceLongUrls->remove($deviceType->value); + } + foreach ($shortUrlEdit->deviceLongUrls as $deviceLongUrlPair) { + $key = $deviceLongUrlPair->deviceType->value; + $deviceLongUrl = $this->deviceLongUrls->get($key); + + if ($deviceLongUrl !== null) { + $deviceLongUrl->updateLongUrl($deviceLongUrlPair->longUrl); + } else { + $this->deviceLongUrls->set($key, DeviceLongUrl::fromShortUrlAndPair($this, $deviceLongUrlPair)); + } + } + } + public function getLongUrl(): string { return $this->longUrl; } + public function longUrlForDevice(?DeviceType $deviceType): string + { + $deviceLongUrl = $deviceType === null ? null : $this->deviceLongUrls->get($deviceType->value); + return $deviceLongUrl?->longUrl() ?? $this->longUrl; + } + public function getShortCode(): string { return $this->shortCode; @@ -218,42 +289,6 @@ class ShortUrl extends AbstractEntity return $this->forwardQuery; } - public function update( - ShortUrlEdition $shortUrlEdit, - ?ShortUrlRelationResolverInterface $relationResolver = null, - ): void { - if ($shortUrlEdit->validSinceWasProvided()) { - $this->validSince = $shortUrlEdit->validSince(); - } - if ($shortUrlEdit->validUntilWasProvided()) { - $this->validUntil = $shortUrlEdit->validUntil(); - } - if ($shortUrlEdit->maxVisitsWasProvided()) { - $this->maxVisits = $shortUrlEdit->maxVisits(); - } - if ($shortUrlEdit->longUrlWasProvided()) { - $this->longUrl = $shortUrlEdit->longUrl() ?? $this->longUrl; - } - if ($shortUrlEdit->tagsWereProvided()) { - $relationResolver = $relationResolver ?? new SimpleShortUrlRelationResolver(); - $this->tags = $relationResolver->resolveTags($shortUrlEdit->tags()); - } - if ($shortUrlEdit->crawlableWasProvided()) { - $this->crawlable = $shortUrlEdit->crawlable(); - } - if ( - $this->title === null - || $shortUrlEdit->titleWasProvided() - || ($this->titleWasAutoResolved && $shortUrlEdit->titleWasAutoResolved()) - ) { - $this->title = $shortUrlEdit->title(); - $this->titleWasAutoResolved = $shortUrlEdit->titleWasAutoResolved(); - } - if ($shortUrlEdit->forwardQueryWasProvided()) { - $this->forwardQuery = $shortUrlEdit->forwardQuery(); - } - } - /** * @throws ShortCodeCannotBeRegeneratedException */ @@ -292,4 +327,14 @@ class ShortUrl extends AbstractEntity return true; } + + public function deviceLongUrls(): array + { + $data = array_fill_keys(enumValues(DeviceType::class), null); + foreach ($this->deviceLongUrls as $deviceUrl) { + $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 f003318d..c322f195 100644 --- a/module/Core/src/ShortUrl/Helper/ShortUrlRedirectionBuilder.php +++ b/module/Core/src/ShortUrl/Helper/ShortUrlRedirectionBuilder.php @@ -7,6 +7,8 @@ namespace Shlinkio\Shlink\Core\ShortUrl\Helper; 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; @@ -14,13 +16,18 @@ use function sprintf; class ShortUrlRedirectionBuilder implements ShortUrlRedirectionBuilderInterface { - public function __construct(private TrackingOptions $trackingOptions) + public function __construct(private readonly TrackingOptions $trackingOptions) { } - public function buildShortUrlRedirect(ShortUrl $shortUrl, array $currentQuery, ?string $extraPath = null): string - { - $uri = Uri::createFromString($shortUrl->getLongUrl()); + public function buildShortUrlRedirect( + ShortUrl $shortUrl, + ServerRequestInterface $request, + ?string $extraPath = null, + ): string { + $currentQuery = $request->getQueryParams(); + $device = DeviceType::matchFromUserAgent($request->getHeaderLine('User-Agent')); + $uri = Uri::createFromString($shortUrl->longUrlForDevice($device)); $shouldForwardQuery = $shortUrl->forwardQuery(); return $uri diff --git a/module/Core/src/ShortUrl/Helper/ShortUrlRedirectionBuilderInterface.php b/module/Core/src/ShortUrl/Helper/ShortUrlRedirectionBuilderInterface.php index 44bd9ccb..7f79e98a 100644 --- a/module/Core/src/ShortUrl/Helper/ShortUrlRedirectionBuilderInterface.php +++ b/module/Core/src/ShortUrl/Helper/ShortUrlRedirectionBuilderInterface.php @@ -4,9 +4,14 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\ShortUrl\Helper; +use Psr\Http\Message\ServerRequestInterface; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; interface ShortUrlRedirectionBuilderInterface { - public function buildShortUrlRedirect(ShortUrl $shortUrl, array $currentQuery, ?string $extraPath = null): string; + public function buildShortUrlRedirect( + ShortUrl $shortUrl, + ServerRequestInterface $request, + ?string $extraPath = null, + ): string; } diff --git a/module/Core/src/ShortUrl/Helper/ShortUrlStringifier.php b/module/Core/src/ShortUrl/Helper/ShortUrlStringifier.php index 719f82b8..9d21cb58 100644 --- a/module/Core/src/ShortUrl/Helper/ShortUrlStringifier.php +++ b/module/Core/src/ShortUrl/Helper/ShortUrlStringifier.php @@ -11,7 +11,7 @@ use function sprintf; class ShortUrlStringifier implements ShortUrlStringifierInterface { - public function __construct(private array $domainConfig, private string $basePath = '') + public function __construct(private readonly array $domainConfig, private readonly string $basePath = '') { } @@ -28,6 +28,6 @@ class ShortUrlStringifier implements ShortUrlStringifierInterface private function resolveDomain(ShortUrl $shortUrl): string { - return $shortUrl->getDomain()?->getAuthority() ?? $this->domainConfig['hostname'] ?? ''; + return $shortUrl->getDomain()?->authority ?? $this->domainConfig['hostname'] ?? ''; } } diff --git a/module/Core/src/ShortUrl/Helper/ShortUrlTitleResolutionHelper.php b/module/Core/src/ShortUrl/Helper/ShortUrlTitleResolutionHelper.php index 00eecc61..a4920cdd 100644 --- a/module/Core/src/ShortUrl/Helper/ShortUrlTitleResolutionHelper.php +++ b/module/Core/src/ShortUrl/Helper/ShortUrlTitleResolutionHelper.php @@ -4,14 +4,21 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\ShortUrl\Helper; +use Shlinkio\Shlink\Core\Exception\InvalidUrlException; use Shlinkio\Shlink\Core\Util\UrlValidatorInterface; class ShortUrlTitleResolutionHelper implements ShortUrlTitleResolutionHelperInterface { - public function __construct(private UrlValidatorInterface $urlValidator) + public function __construct(private readonly UrlValidatorInterface $urlValidator) { } + /** + * @template T of TitleResolutionModelInterface + * @param T $data + * @return T + * @throws InvalidUrlException + */ public function processTitleAndValidateUrl(TitleResolutionModelInterface $data): TitleResolutionModelInterface { if ($data->hasTitle()) { diff --git a/module/Core/src/ShortUrl/Helper/ShortUrlTitleResolutionHelperInterface.php b/module/Core/src/ShortUrl/Helper/ShortUrlTitleResolutionHelperInterface.php index 50022746..6989140a 100644 --- a/module/Core/src/ShortUrl/Helper/ShortUrlTitleResolutionHelperInterface.php +++ b/module/Core/src/ShortUrl/Helper/ShortUrlTitleResolutionHelperInterface.php @@ -9,6 +9,9 @@ use Shlinkio\Shlink\Core\Exception\InvalidUrlException; interface ShortUrlTitleResolutionHelperInterface { /** + * @template T of TitleResolutionModelInterface + * @param T $data + * @return T * @throws InvalidUrlException */ public function processTitleAndValidateUrl(TitleResolutionModelInterface $data): TitleResolutionModelInterface; diff --git a/module/Core/src/ShortUrl/Helper/TitleResolutionModelInterface.php b/module/Core/src/ShortUrl/Helper/TitleResolutionModelInterface.php index 8af28706..1c834331 100644 --- a/module/Core/src/ShortUrl/Helper/TitleResolutionModelInterface.php +++ b/module/Core/src/ShortUrl/Helper/TitleResolutionModelInterface.php @@ -12,5 +12,5 @@ interface TitleResolutionModelInterface public function doValidateUrl(): bool; - public function withResolvedTitle(string $title): self; + public function withResolvedTitle(string $title): static; } diff --git a/module/Core/src/ShortUrl/Middleware/ExtraPathRedirectMiddleware.php b/module/Core/src/ShortUrl/Middleware/ExtraPathRedirectMiddleware.php index 66105779..c8f96bba 100644 --- a/module/Core/src/ShortUrl/Middleware/ExtraPathRedirectMiddleware.php +++ b/module/Core/src/ShortUrl/Middleware/ExtraPathRedirectMiddleware.php @@ -68,7 +68,6 @@ class ExtraPathRedirectMiddleware implements MiddlewareInterface int $shortCodeSegments = 1, ): ResponseInterface { $uri = $request->getUri(); - $query = $request->getQueryParams(); [$potentialShortCode, $extraPath] = $this->resolvePotentialShortCodeAndExtraPath($uri, $shortCodeSegments); $identifier = ShortUrlIdentifier::fromShortCodeAndDomain($potentialShortCode, $uri->getAuthority()); @@ -76,7 +75,7 @@ class ExtraPathRedirectMiddleware implements MiddlewareInterface $shortUrl = $this->resolver->resolveEnabledShortUrl($identifier); $this->requestTracker->trackIfApplicable($shortUrl, $request); - $longUrl = $this->redirectionBuilder->buildShortUrlRedirect($shortUrl, $query, $extraPath); + $longUrl = $this->redirectionBuilder->buildShortUrlRedirect($shortUrl, $request, $extraPath); return $this->redirectResponseHelper->buildRedirectResponse($longUrl); } catch (ShortUrlNotFoundException) { if ($extraPath === null || ! $this->urlShortenerOptions->multiSegmentSlugsEnabled) { diff --git a/module/Core/src/ShortUrl/Model/DeviceLongUrlPair.php b/module/Core/src/ShortUrl/Model/DeviceLongUrlPair.php new file mode 100644 index 00000000..d017c7e5 --- /dev/null +++ b/module/Core/src/ShortUrl/Model/DeviceLongUrlPair.php @@ -0,0 +1,47 @@ + $map + * @return array{array, DeviceType[]} + */ + public static function fromMapToChangeSet(array $map): array + { + $typesWithNullUrl = group($map, static fn (?string $longUrl) => $longUrl === null ? 'remove' : 'keep'); + $deviceTypesToRemove = array_values(map( + $typesWithNullUrl['remove'] ?? [], + static fn ($_, string $deviceType) => DeviceType::from($deviceType), + )); + $pairsToKeep = map( + $typesWithNullUrl['keep'] ?? [], + fn (string $longUrl, string $deviceType) => self::fromRawTypeAndLongUrl($deviceType, $longUrl), + ); + + return [$pairsToKeep, $deviceTypesToRemove]; + } +} diff --git a/module/Core/src/ShortUrl/Model/OrderableField.php b/module/Core/src/ShortUrl/Model/OrderableField.php index 1c1c6338..ac1bc632 100644 --- a/module/Core/src/ShortUrl/Model/OrderableField.php +++ b/module/Core/src/ShortUrl/Model/OrderableField.php @@ -3,7 +3,6 @@ namespace Shlinkio\Shlink\Core\ShortUrl\Model; use function Functional\contains; -use function Functional\map; enum OrderableField: string { @@ -14,14 +13,6 @@ enum OrderableField: string case VISITS = 'visits'; case NON_BOT_VISITS = 'nonBotVisits'; - /** - * @return string[] - */ - public static function values(): array - { - return map(self::cases(), static fn (OrderableField $field) => $field->value); - } - public static function isBasicField(string $value): bool { return contains( diff --git a/module/Core/src/ShortUrl/Model/ShortUrlCreation.php b/module/Core/src/ShortUrl/Model/ShortUrlCreation.php index bbdd9ab0..d5078f7b 100644 --- a/module/Core/src/ShortUrl/Model/ShortUrlCreation.php +++ b/module/Core/src/ShortUrl/Model/ShortUrlCreation.php @@ -19,72 +19,86 @@ use const Shlinkio\Shlink\DEFAULT_SHORT_CODES_LENGTH; final class ShortUrlCreation implements TitleResolutionModelInterface { - private string $longUrl; - private ?Chronos $validSince = null; - private ?Chronos $validUntil = null; - private ?string $customSlug = null; - private ?int $maxVisits = null; - private ?bool $findIfExists = null; - private ?string $domain = null; - private int $shortCodeLength = 5; - private bool $validateUrl = false; - private ?ApiKey $apiKey = null; - private array $tags = []; - private ?string $title = null; - private bool $titleWasAutoResolved = false; - private bool $crawlable = false; - private bool $forwardQuery = true; - - private function __construct() - { - } - - public static function createEmpty(): self - { - $instance = new self(); - $instance->longUrl = ''; - - return $instance; + /** + * @param string[] $tags + * @param DeviceLongUrlPair[] $deviceLongUrls + */ + private function __construct( + public readonly string $longUrl, + public readonly array $deviceLongUrls = [], + public readonly ?Chronos $validSince = null, + public readonly ?Chronos $validUntil = null, + public readonly ?string $customSlug = null, + public readonly ?int $maxVisits = null, + public readonly bool $findIfExists = false, + public readonly ?string $domain = null, + public readonly int $shortCodeLength = 5, + public readonly bool $validateUrl = false, + public readonly ?ApiKey $apiKey = null, + public readonly array $tags = [], + public readonly ?string $title = null, + public readonly bool $titleWasAutoResolved = false, + public readonly bool $crawlable = false, + public readonly bool $forwardQuery = true, + ) { } /** * @throws ValidationException */ public static function fromRawData(array $data): self - { - $instance = new self(); - $instance->validateAndInit($data); - - return $instance; - } - - /** - * @throws ValidationException - */ - private function validateAndInit(array $data): void { $inputFilter = ShortUrlInputFilter::withRequiredLongUrl($data); if (! $inputFilter->isValid()) { throw ValidationException::fromInputFilter($inputFilter); } - $this->longUrl = $inputFilter->getValue(ShortUrlInputFilter::LONG_URL); - $this->validSince = normalizeOptionalDate($inputFilter->getValue(ShortUrlInputFilter::VALID_SINCE)); - $this->validUntil = normalizeOptionalDate($inputFilter->getValue(ShortUrlInputFilter::VALID_UNTIL)); - $this->customSlug = $inputFilter->getValue(ShortUrlInputFilter::CUSTOM_SLUG); - $this->maxVisits = getOptionalIntFromInputFilter($inputFilter, ShortUrlInputFilter::MAX_VISITS); - $this->findIfExists = $inputFilter->getValue(ShortUrlInputFilter::FIND_IF_EXISTS); - $this->validateUrl = getOptionalBoolFromInputFilter($inputFilter, ShortUrlInputFilter::VALIDATE_URL) ?? false; - $this->domain = getNonEmptyOptionalValueFromInputFilter($inputFilter, ShortUrlInputFilter::DOMAIN); - $this->shortCodeLength = getOptionalIntFromInputFilter( - $inputFilter, - ShortUrlInputFilter::SHORT_CODE_LENGTH, - ) ?? DEFAULT_SHORT_CODES_LENGTH; - $this->apiKey = $inputFilter->getValue(ShortUrlInputFilter::API_KEY); - $this->tags = $inputFilter->getValue(ShortUrlInputFilter::TAGS); - $this->title = $inputFilter->getValue(ShortUrlInputFilter::TITLE); - $this->crawlable = $inputFilter->getValue(ShortUrlInputFilter::CRAWLABLE); - $this->forwardQuery = getOptionalBoolFromInputFilter($inputFilter, ShortUrlInputFilter::FORWARD_QUERY) ?? true; + [$deviceLongUrls] = DeviceLongUrlPair::fromMapToChangeSet( + $inputFilter->getValue(ShortUrlInputFilter::DEVICE_LONG_URLS) ?? [], + ); + + return new self( + longUrl: $inputFilter->getValue(ShortUrlInputFilter::LONG_URL), + deviceLongUrls: $deviceLongUrls, + validSince: normalizeOptionalDate($inputFilter->getValue(ShortUrlInputFilter::VALID_SINCE)), + validUntil: normalizeOptionalDate($inputFilter->getValue(ShortUrlInputFilter::VALID_UNTIL)), + customSlug: $inputFilter->getValue(ShortUrlInputFilter::CUSTOM_SLUG), + maxVisits: getOptionalIntFromInputFilter($inputFilter, ShortUrlInputFilter::MAX_VISITS), + findIfExists: $inputFilter->getValue(ShortUrlInputFilter::FIND_IF_EXISTS) ?? false, + domain: getNonEmptyOptionalValueFromInputFilter($inputFilter, ShortUrlInputFilter::DOMAIN), + shortCodeLength: getOptionalIntFromInputFilter( + $inputFilter, + ShortUrlInputFilter::SHORT_CODE_LENGTH, + ) ?? DEFAULT_SHORT_CODES_LENGTH, + validateUrl: getOptionalBoolFromInputFilter($inputFilter, ShortUrlInputFilter::VALIDATE_URL) ?? false, + apiKey: $inputFilter->getValue(ShortUrlInputFilter::API_KEY), + tags: $inputFilter->getValue(ShortUrlInputFilter::TAGS), + title: $inputFilter->getValue(ShortUrlInputFilter::TITLE), + crawlable: $inputFilter->getValue(ShortUrlInputFilter::CRAWLABLE), + forwardQuery: getOptionalBoolFromInputFilter($inputFilter, ShortUrlInputFilter::FORWARD_QUERY) ?? true, + ); + } + + public function withResolvedTitle(string $title): static + { + return new self( + longUrl: $this->longUrl, + deviceLongUrls: $this->deviceLongUrls, + validSince: $this->validSince, + validUntil: $this->validUntil, + customSlug: $this->customSlug, + maxVisits: $this->maxVisits, + findIfExists: $this->findIfExists, + domain: $this->domain, + shortCodeLength: $this->shortCodeLength, + validateUrl: $this->validateUrl, + apiKey: $this->apiKey, + tags: $this->tags, + title: $title, + titleWasAutoResolved: true, + crawlable: $this->crawlable, + forwardQuery: $this->forwardQuery, + ); } public function getLongUrl(): string @@ -92,115 +106,38 @@ final class ShortUrlCreation implements TitleResolutionModelInterface return $this->longUrl; } - public function getValidSince(): ?Chronos - { - return $this->validSince; - } - public function hasValidSince(): bool { return $this->validSince !== null; } - public function getValidUntil(): ?Chronos - { - return $this->validUntil; - } - public function hasValidUntil(): bool { return $this->validUntil !== null; } - public function getCustomSlug(): ?string - { - return $this->customSlug; - } - public function hasCustomSlug(): bool { return $this->customSlug !== null; } - public function getMaxVisits(): ?int - { - return $this->maxVisits; - } - public function hasMaxVisits(): bool { return $this->maxVisits !== null; } - public function findIfExists(): bool - { - return (bool) $this->findIfExists; - } - public function hasDomain(): bool { return $this->domain !== null; } - public function getDomain(): ?string - { - return $this->domain; - } - - public function getShortCodeLength(): int - { - return $this->shortCodeLength; - } - public function doValidateUrl(): bool { return $this->validateUrl; } - public function getApiKey(): ?ApiKey - { - return $this->apiKey; - } - - /** - * @return string[] - */ - public function getTags(): array - { - return $this->tags; - } - - public function getTitle(): ?string - { - return $this->title; - } - public function hasTitle(): bool { return $this->title !== null; } - - public function titleWasAutoResolved(): bool - { - return $this->titleWasAutoResolved; - } - - public function withResolvedTitle(string $title): self - { - $copy = clone $this; - $copy->title = $title; - $copy->titleWasAutoResolved = true; - - return $copy; - } - - public function isCrawlable(): bool - { - return $this->crawlable; - } - - public function forwardQuery(): bool - { - return $this->forwardQuery; - } } diff --git a/module/Core/src/ShortUrl/Model/ShortUrlEdition.php b/module/Core/src/ShortUrl/Model/ShortUrlEdition.php index fadc9b1e..6bc157c7 100644 --- a/module/Core/src/ShortUrl/Model/ShortUrlEdition.php +++ b/module/Core/src/ShortUrl/Model/ShortUrlEdition.php @@ -6,6 +6,7 @@ namespace Shlinkio\Shlink\Core\ShortUrl\Model; use Cake\Chronos\Chronos; use Shlinkio\Shlink\Core\Exception\ValidationException; +use Shlinkio\Shlink\Core\Model\DeviceType; use Shlinkio\Shlink\Core\ShortUrl\Helper\TitleResolutionModelInterface; use Shlinkio\Shlink\Core\ShortUrl\Model\Validation\ShortUrlInputFilter; @@ -16,77 +17,101 @@ use function Shlinkio\Shlink\Core\normalizeOptionalDate; final class ShortUrlEdition implements TitleResolutionModelInterface { - private bool $longUrlPropWasProvided = false; - private ?string $longUrl = null; - private bool $validSincePropWasProvided = false; - private ?Chronos $validSince = null; - private bool $validUntilPropWasProvided = false; - private ?Chronos $validUntil = null; - private bool $maxVisitsPropWasProvided = false; - private ?int $maxVisits = null; - private bool $tagsPropWasProvided = false; - private array $tags = []; - private bool $titlePropWasProvided = false; - private ?string $title = null; - private bool $titleWasAutoResolved = false; - private bool $validateUrl = false; - private bool $crawlablePropWasProvided = false; - private bool $crawlable = false; - private bool $forwardQueryPropWasProvided = false; - private bool $forwardQuery = true; - - private function __construct() - { + /** + * @param string[] $tags + * @param DeviceLongUrlPair[] $deviceLongUrls + * @param DeviceType[] $devicesToRemove + */ + private function __construct( + private readonly bool $longUrlPropWasProvided = false, + public readonly ?string $longUrl = null, + public readonly array $deviceLongUrls = [], + public readonly array $devicesToRemove = [], + private readonly bool $validSincePropWasProvided = false, + public readonly ?Chronos $validSince = null, + private readonly bool $validUntilPropWasProvided = false, + public readonly ?Chronos $validUntil = null, + private readonly bool $maxVisitsPropWasProvided = false, + public readonly ?int $maxVisits = null, + private readonly bool $tagsPropWasProvided = false, + public readonly array $tags = [], + private readonly bool $titlePropWasProvided = false, + public readonly ?string $title = null, + public readonly bool $titleWasAutoResolved = false, + public readonly bool $validateUrl = false, + private readonly bool $crawlablePropWasProvided = false, + public readonly bool $crawlable = false, + private readonly bool $forwardQueryPropWasProvided = false, + public readonly bool $forwardQuery = true, + ) { } /** * @throws ValidationException */ public static function fromRawData(array $data): self - { - $instance = new self(); - $instance->validateAndInit($data); - return $instance; - } - - /** - * @throws ValidationException - */ - private function validateAndInit(array $data): void { $inputFilter = ShortUrlInputFilter::withNonRequiredLongUrl($data); if (! $inputFilter->isValid()) { throw ValidationException::fromInputFilter($inputFilter); } - $this->longUrlPropWasProvided = array_key_exists(ShortUrlInputFilter::LONG_URL, $data); - $this->validSincePropWasProvided = array_key_exists(ShortUrlInputFilter::VALID_SINCE, $data); - $this->validUntilPropWasProvided = array_key_exists(ShortUrlInputFilter::VALID_UNTIL, $data); - $this->maxVisitsPropWasProvided = array_key_exists(ShortUrlInputFilter::MAX_VISITS, $data); - $this->tagsPropWasProvided = array_key_exists(ShortUrlInputFilter::TAGS, $data); - $this->titlePropWasProvided = array_key_exists(ShortUrlInputFilter::TITLE, $data); - $this->crawlablePropWasProvided = array_key_exists(ShortUrlInputFilter::CRAWLABLE, $data); - $this->forwardQueryPropWasProvided = array_key_exists(ShortUrlInputFilter::FORWARD_QUERY, $data); + [$deviceLongUrls, $devicesToRemove] = DeviceLongUrlPair::fromMapToChangeSet( + $inputFilter->getValue(ShortUrlInputFilter::DEVICE_LONG_URLS) ?? [], + ); - $this->longUrl = $inputFilter->getValue(ShortUrlInputFilter::LONG_URL); - $this->validSince = normalizeOptionalDate($inputFilter->getValue(ShortUrlInputFilter::VALID_SINCE)); - $this->validUntil = normalizeOptionalDate($inputFilter->getValue(ShortUrlInputFilter::VALID_UNTIL)); - $this->maxVisits = getOptionalIntFromInputFilter($inputFilter, ShortUrlInputFilter::MAX_VISITS); - $this->validateUrl = getOptionalBoolFromInputFilter($inputFilter, ShortUrlInputFilter::VALIDATE_URL) ?? false; - $this->tags = $inputFilter->getValue(ShortUrlInputFilter::TAGS); - $this->title = $inputFilter->getValue(ShortUrlInputFilter::TITLE); - $this->crawlable = $inputFilter->getValue(ShortUrlInputFilter::CRAWLABLE); - $this->forwardQuery = getOptionalBoolFromInputFilter($inputFilter, ShortUrlInputFilter::FORWARD_QUERY) ?? true; + return new self( + longUrlPropWasProvided: array_key_exists(ShortUrlInputFilter::LONG_URL, $data), + longUrl: $inputFilter->getValue(ShortUrlInputFilter::LONG_URL), + deviceLongUrls: $deviceLongUrls, + devicesToRemove: $devicesToRemove, + validSincePropWasProvided: array_key_exists(ShortUrlInputFilter::VALID_SINCE, $data), + validSince: normalizeOptionalDate($inputFilter->getValue(ShortUrlInputFilter::VALID_SINCE)), + validUntilPropWasProvided: array_key_exists(ShortUrlInputFilter::VALID_UNTIL, $data), + validUntil: normalizeOptionalDate($inputFilter->getValue(ShortUrlInputFilter::VALID_UNTIL)), + maxVisitsPropWasProvided: array_key_exists(ShortUrlInputFilter::MAX_VISITS, $data), + maxVisits: getOptionalIntFromInputFilter($inputFilter, ShortUrlInputFilter::MAX_VISITS), + tagsPropWasProvided: array_key_exists(ShortUrlInputFilter::TAGS, $data), + tags: $inputFilter->getValue(ShortUrlInputFilter::TAGS), + titlePropWasProvided: array_key_exists(ShortUrlInputFilter::TITLE, $data), + title: $inputFilter->getValue(ShortUrlInputFilter::TITLE), + validateUrl: getOptionalBoolFromInputFilter($inputFilter, ShortUrlInputFilter::VALIDATE_URL) ?? false, + crawlablePropWasProvided: array_key_exists(ShortUrlInputFilter::CRAWLABLE, $data), + crawlable: $inputFilter->getValue(ShortUrlInputFilter::CRAWLABLE), + forwardQueryPropWasProvided: array_key_exists(ShortUrlInputFilter::FORWARD_QUERY, $data), + forwardQuery: getOptionalBoolFromInputFilter($inputFilter, ShortUrlInputFilter::FORWARD_QUERY) ?? true, + ); } - public function longUrl(): ?string + public function withResolvedTitle(string $title): static { - return $this->longUrl; + return new self( + longUrlPropWasProvided: $this->longUrlPropWasProvided, + longUrl: $this->longUrl, + deviceLongUrls: $this->deviceLongUrls, + devicesToRemove: $this->devicesToRemove, + validSincePropWasProvided: $this->validSincePropWasProvided, + validSince: $this->validSince, + validUntilPropWasProvided: $this->validUntilPropWasProvided, + validUntil: $this->validUntil, + maxVisitsPropWasProvided: $this->maxVisitsPropWasProvided, + maxVisits: $this->maxVisits, + tagsPropWasProvided: $this->tagsPropWasProvided, + tags: $this->tags, + titlePropWasProvided: $this->titlePropWasProvided, + title: $title, + titleWasAutoResolved: true, + validateUrl: $this->validateUrl, + crawlablePropWasProvided: $this->crawlablePropWasProvided, + crawlable: $this->crawlable, + forwardQueryPropWasProvided: $this->forwardQueryPropWasProvided, + forwardQuery: $this->forwardQuery, + ); } public function getLongUrl(): string { - return $this->longUrl() ?? ''; + return $this->longUrl ?? ''; } public function longUrlWasProvided(): bool @@ -94,54 +119,26 @@ final class ShortUrlEdition implements TitleResolutionModelInterface return $this->longUrlPropWasProvided && $this->longUrl !== null; } - public function validSince(): ?Chronos - { - return $this->validSince; - } - public function validSinceWasProvided(): bool { return $this->validSincePropWasProvided; } - public function validUntil(): ?Chronos - { - return $this->validUntil; - } - public function validUntilWasProvided(): bool { return $this->validUntilPropWasProvided; } - public function maxVisits(): ?int - { - return $this->maxVisits; - } - public function maxVisitsWasProvided(): bool { return $this->maxVisitsPropWasProvided; } - /** - * @return string[] - */ - public function tags(): array - { - return $this->tags; - } - public function tagsWereProvided(): bool { return $this->tagsPropWasProvided; } - public function title(): ?string - { - return $this->title; - } - public function titleWasProvided(): bool { return $this->titlePropWasProvided; @@ -157,35 +154,16 @@ final class ShortUrlEdition implements TitleResolutionModelInterface return $this->titleWasAutoResolved; } - public function withResolvedTitle(string $title): self - { - $copy = clone $this; - $copy->title = $title; - $copy->titleWasAutoResolved = true; - - return $copy; - } - public function doValidateUrl(): bool { return $this->validateUrl; } - public function crawlable(): bool - { - return $this->crawlable; - } - public function crawlableWasProvided(): bool { return $this->crawlablePropWasProvided; } - public function forwardQuery(): bool - { - return $this->forwardQuery; - } - public function forwardQueryWasProvided(): bool { return $this->forwardQueryPropWasProvided; diff --git a/module/Core/src/ShortUrl/Model/ShortUrlIdentifier.php b/module/Core/src/ShortUrl/Model/ShortUrlIdentifier.php index d7b49c68..bb3b4af6 100644 --- a/module/Core/src/ShortUrl/Model/ShortUrlIdentifier.php +++ b/module/Core/src/ShortUrl/Model/ShortUrlIdentifier.php @@ -45,7 +45,7 @@ final class ShortUrlIdentifier public static function fromShortUrl(ShortUrl $shortUrl): self { $domain = $shortUrl->getDomain(); - $domainAuthority = $domain?->getAuthority(); + $domainAuthority = $domain?->authority; return new self($shortUrl->getShortCode(), $domainAuthority); } diff --git a/module/Core/src/ShortUrl/Model/TagsMode.php b/module/Core/src/ShortUrl/Model/TagsMode.php index 01cdcc3b..593d6d83 100644 --- a/module/Core/src/ShortUrl/Model/TagsMode.php +++ b/module/Core/src/ShortUrl/Model/TagsMode.php @@ -4,15 +4,8 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\ShortUrl\Model; -use function Functional\map; - enum TagsMode: string { case ANY = 'any'; case ALL = 'all'; - - public static function values(): array - { - return map(self::cases(), static fn (TagsMode $mode) => $mode->value); - } } diff --git a/module/Core/src/ShortUrl/Model/Validation/DeviceLongUrlsValidator.php b/module/Core/src/ShortUrl/Model/Validation/DeviceLongUrlsValidator.php new file mode 100644 index 00000000..9fda1809 --- /dev/null +++ b/module/Core/src/ShortUrl/Model/Validation/DeviceLongUrlsValidator.php @@ -0,0 +1,57 @@ + 'Provided value is not an array.', + self::INVALID_DEVICE => 'You have provided at least one invalid device identifier.', + self::INVALID_LONG_URL => 'At least one of the long URLs are invalid.', + ]; + + public function __construct(private readonly ValidatorInterface $longUrlValidators) + { + parent::__construct(); + } + + public function isValid(mixed $value): bool + { + if (! is_array($value)) { + $this->error(self::NOT_ARRAY); + return false; + } + + $validValues = enumValues(DeviceType::class); + $keys = array_keys($value); + if (! every($keys, static fn ($key) => contains($validValues, $key))) { + $this->error(self::INVALID_DEVICE); + return false; + } + + $longUrls = array_values($value); + $result = every($longUrls, $this->longUrlValidators->isValid(...)); + if (! $result) { + $this->error(self::INVALID_LONG_URL); + } + + return $result; + } +} diff --git a/module/Core/src/ShortUrl/Model/Validation/ShortUrlInputFilter.php b/module/Core/src/ShortUrl/Model/Validation/ShortUrlInputFilter.php index a6c5627f..4a0e2d7b 100644 --- a/module/Core/src/ShortUrl/Model/Validation/ShortUrlInputFilter.php +++ b/module/Core/src/ShortUrl/Model/Validation/ShortUrlInputFilter.php @@ -4,9 +4,8 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\ShortUrl\Model\Validation; -use DateTime; +use DateTimeInterface; use Laminas\Filter; -use Laminas\InputFilter\Input; use Laminas\InputFilter\InputFilter; use Laminas\Validator; use Shlinkio\Shlink\Common\Validation; @@ -32,6 +31,7 @@ class ShortUrlInputFilter extends InputFilter public const DOMAIN = 'domain'; public const SHORT_CODE_LENGTH = 'shortCodeLength'; public const LONG_URL = 'longUrl'; + public const DEVICE_LONG_URLS = 'deviceLongUrls'; public const VALIDATE_URL = 'validateUrl'; public const API_KEY = 'apiKey'; public const TAGS = 'tags'; @@ -41,6 +41,7 @@ class ShortUrlInputFilter extends InputFilter private function __construct(array $data, bool $requireLongUrl) { + // FIXME The multi-segment slug option should be injected $this->initialize($requireLongUrl, $data[EnvVars::MULTI_SEGMENT_SLUGS_ENABLED->value] ?? false); $this->setData($data); } @@ -57,26 +58,40 @@ class ShortUrlInputFilter extends InputFilter private function initialize(bool $requireLongUrl, bool $multiSegmentEnabled): void { - $longUrlInput = $this->createInput(self::LONG_URL, $requireLongUrl); - $longUrlInput->getValidatorChain()->attach(new Validator\NotEmpty([ + $longUrlNotEmptyCommonOptions = [ Validator\NotEmpty::OBJECT, Validator\NotEmpty::SPACE, - Validator\NotEmpty::NULL, Validator\NotEmpty::EMPTY_ARRAY, Validator\NotEmpty::BOOLEAN, + Validator\NotEmpty::STRING, + ]; + + $longUrlInput = $this->createInput(self::LONG_URL, $requireLongUrl); + $longUrlInput->getValidatorChain()->attach(new Validator\NotEmpty([ + ...$longUrlNotEmptyCommonOptions, + Validator\NotEmpty::NULL, ])); $this->add($longUrlInput); + $deviceLongUrlsInput = $this->createInput(self::DEVICE_LONG_URLS, false); + $deviceLongUrlsInput->getValidatorChain()->attach( + new DeviceLongUrlsValidator(new Validator\NotEmpty([ + ...$longUrlNotEmptyCommonOptions, + ...($requireLongUrl ? [Validator\NotEmpty::NULL] : []), + ])), + ); + $this->add($deviceLongUrlsInput); + $validSince = $this->createInput(self::VALID_SINCE, false); - $validSince->getValidatorChain()->attach(new Validator\Date(['format' => DateTime::ATOM])); + $validSince->getValidatorChain()->attach(new Validator\Date(['format' => DateTimeInterface::ATOM])); $this->add($validSince); $validUntil = $this->createInput(self::VALID_UNTIL, false); - $validUntil->getValidatorChain()->attach(new Validator\Date(['format' => DateTime::ATOM])); + $validUntil->getValidatorChain()->attach(new Validator\Date(['format' => DateTimeInterface::ATOM])); $this->add($validUntil); - // FIXME The only way to enforce the NotEmpty validator to be evaluated when the value is provided but it's - // empty, is by using the deprecated setContinueIfEmpty + // The only way to enforce the NotEmpty validator to be evaluated when the key is present with an empty value + // is by using the deprecated setContinueIfEmpty $customSlug = $this->createInput(self::CUSTOM_SLUG, false)->setContinueIfEmpty(true); $customSlug->getFilterChain()->attach(new Filter\Callback(match ($multiSegmentEnabled) { true => static fn (mixed $v) => is_string($v) ? trim(str_replace(' ', '-', $v), '/') : $v, @@ -102,10 +117,8 @@ class ShortUrlInputFilter extends InputFilter $domain->getValidatorChain()->attach(new Validation\HostAndPortValidator()); $this->add($domain); - $apiKeyInput = new Input(self::API_KEY); - $apiKeyInput - ->setRequired(false) - ->getValidatorChain()->attach(new Validator\IsInstanceOf(['className' => ApiKey::class])); + $apiKeyInput = $this->createInput(self::API_KEY, false); + $apiKeyInput->getValidatorChain()->attach(new Validator\IsInstanceOf(['className' => ApiKey::class])); $this->add($apiKeyInput); $this->add($this->createTagsInput(self::TAGS, false)); diff --git a/module/Core/src/ShortUrl/Model/Validation/ShortUrlsParamsInputFilter.php b/module/Core/src/ShortUrl/Model/Validation/ShortUrlsParamsInputFilter.php index cb120e8e..d7cda41e 100644 --- a/module/Core/src/ShortUrl/Model/Validation/ShortUrlsParamsInputFilter.php +++ b/module/Core/src/ShortUrl/Model/Validation/ShortUrlsParamsInputFilter.php @@ -11,6 +11,8 @@ use Shlinkio\Shlink\Common\Validation; use Shlinkio\Shlink\Core\ShortUrl\Model\OrderableField; use Shlinkio\Shlink\Core\ShortUrl\Model\TagsMode; +use function Shlinkio\Shlink\Core\enumValues; + class ShortUrlsParamsInputFilter extends InputFilter { use Validation\InputFactoryTrait; @@ -46,12 +48,12 @@ class ShortUrlsParamsInputFilter extends InputFilter $tagsMode = $this->createInput(self::TAGS_MODE, false); $tagsMode->getValidatorChain()->attach(new InArray([ - 'haystack' => TagsMode::values(), + 'haystack' => enumValues(TagsMode::class), 'strict' => InArray::COMPARE_STRICT, ])); $this->add($tagsMode); - $this->add($this->createOrderByInput(self::ORDER_BY, OrderableField::values())); + $this->add($this->createOrderByInput(self::ORDER_BY, enumValues(OrderableField::class))); $this->add($this->createBooleanInput(self::EXCLUDE_MAX_VISITS_REACHED, false)); $this->add($this->createBooleanInput(self::EXCLUDE_PAST_VALID_UNTIL, false)); diff --git a/module/Core/src/ShortUrl/Repository/ShortUrlRepository.php b/module/Core/src/ShortUrl/Repository/ShortUrlRepository.php index 5e95f777..ee2f7389 100644 --- a/module/Core/src/ShortUrl/Repository/ShortUrlRepository.php +++ b/module/Core/src/ShortUrl/Repository/ShortUrlRepository.php @@ -101,45 +101,45 @@ class ShortUrlRepository extends EntitySpecificationRepository implements ShortU return $qb; } - public function findOneMatching(ShortUrlCreation $meta): ?ShortUrl + public function findOneMatching(ShortUrlCreation $creation): ?ShortUrl { $qb = $this->getEntityManager()->createQueryBuilder(); $qb->select('s') ->from(ShortUrl::class, 's') ->where($qb->expr()->eq('s.longUrl', ':longUrl')) - ->setParameter('longUrl', $meta->getLongUrl()) + ->setParameter('longUrl', $creation->longUrl) ->setMaxResults(1) ->orderBy('s.id'); - if ($meta->hasCustomSlug()) { + if ($creation->hasCustomSlug()) { $qb->andWhere($qb->expr()->eq('s.shortCode', ':slug')) - ->setParameter('slug', $meta->getCustomSlug()); + ->setParameter('slug', $creation->customSlug); } - if ($meta->hasMaxVisits()) { + if ($creation->hasMaxVisits()) { $qb->andWhere($qb->expr()->eq('s.maxVisits', ':maxVisits')) - ->setParameter('maxVisits', $meta->getMaxVisits()); + ->setParameter('maxVisits', $creation->maxVisits); } - if ($meta->hasValidSince()) { + if ($creation->hasValidSince()) { $qb->andWhere($qb->expr()->eq('s.validSince', ':validSince')) - ->setParameter('validSince', $meta->getValidSince(), ChronosDateTimeType::CHRONOS_DATETIME); + ->setParameter('validSince', $creation->validSince, ChronosDateTimeType::CHRONOS_DATETIME); } - if ($meta->hasValidUntil()) { + if ($creation->hasValidUntil()) { $qb->andWhere($qb->expr()->eq('s.validUntil', ':validUntil')) - ->setParameter('validUntil', $meta->getValidUntil(), ChronosDateTimeType::CHRONOS_DATETIME); + ->setParameter('validUntil', $creation->validUntil, ChronosDateTimeType::CHRONOS_DATETIME); } - if ($meta->hasDomain()) { + if ($creation->hasDomain()) { $qb->join('s.domain', 'd') ->andWhere($qb->expr()->eq('d.authority', ':domain')) - ->setParameter('domain', $meta->getDomain()); + ->setParameter('domain', $creation->domain); } - $apiKey = $meta->getApiKey(); + $apiKey = $creation->apiKey; if ($apiKey !== null) { $this->applySpecification($qb, $apiKey->spec(), 's'); } - $tags = $meta->getTags(); + $tags = $creation->tags; $tagsAmount = count($tags); if ($tagsAmount === 0) { return $qb->getQuery()->getOneOrNullResult(); diff --git a/module/Core/src/ShortUrl/Repository/ShortUrlRepositoryInterface.php b/module/Core/src/ShortUrl/Repository/ShortUrlRepositoryInterface.php index cc574ac5..18a4ec71 100644 --- a/module/Core/src/ShortUrl/Repository/ShortUrlRepositoryInterface.php +++ b/module/Core/src/ShortUrl/Repository/ShortUrlRepositoryInterface.php @@ -22,7 +22,7 @@ interface ShortUrlRepositoryInterface extends ObjectRepository, EntitySpecificat public function shortCodeIsInUseWithLock(ShortUrlIdentifier $identifier, ?Specification $spec = null): bool; - public function findOneMatching(ShortUrlCreation $meta): ?ShortUrl; + public function findOneMatching(ShortUrlCreation $creation): ?ShortUrl; public function findOneByImportedUrl(ImportedShlinkUrl $url): ?ShortUrl; } diff --git a/module/Core/src/ShortUrl/Resolver/PersistenceShortUrlRelationResolver.php b/module/Core/src/ShortUrl/Resolver/PersistenceShortUrlRelationResolver.php index 971ef932..db6721d5 100644 --- a/module/Core/src/ShortUrl/Resolver/PersistenceShortUrlRelationResolver.php +++ b/module/Core/src/ShortUrl/Resolver/PersistenceShortUrlRelationResolver.php @@ -49,7 +49,7 @@ class PersistenceShortUrlRelationResolver implements ShortUrlRelationResolverInt /** * @param string[] $tags - * @return Collection|Tag[] + * @return Collection */ public function resolveTags(array $tags): Collections\Collection { diff --git a/module/Core/src/ShortUrl/Resolver/ShortUrlRelationResolverInterface.php b/module/Core/src/ShortUrl/Resolver/ShortUrlRelationResolverInterface.php index a71f2ccc..b5228214 100644 --- a/module/Core/src/ShortUrl/Resolver/ShortUrlRelationResolverInterface.php +++ b/module/Core/src/ShortUrl/Resolver/ShortUrlRelationResolverInterface.php @@ -14,7 +14,7 @@ interface ShortUrlRelationResolverInterface /** * @param string[] $tags - * @return Collection|Tag[] + * @return Collection */ public function resolveTags(array $tags): Collection; } diff --git a/module/Core/src/ShortUrl/Resolver/SimpleShortUrlRelationResolver.php b/module/Core/src/ShortUrl/Resolver/SimpleShortUrlRelationResolver.php index f25ff8a1..609a300c 100644 --- a/module/Core/src/ShortUrl/Resolver/SimpleShortUrlRelationResolver.php +++ b/module/Core/src/ShortUrl/Resolver/SimpleShortUrlRelationResolver.php @@ -5,7 +5,6 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\ShortUrl\Resolver; use Doctrine\Common\Collections; -use Doctrine\Common\Collections\Collection; use Shlinkio\Shlink\Core\Domain\Entity\Domain; use Shlinkio\Shlink\Core\Tag\Entity\Tag; @@ -20,7 +19,7 @@ class SimpleShortUrlRelationResolver implements ShortUrlRelationResolverInterfac /** * @param string[] $tags - * @return Collection|Tag[] + * @return Collections\Collection */ public function resolveTags(array $tags): Collections\Collection { diff --git a/module/Core/src/ShortUrl/ShortUrlService.php b/module/Core/src/ShortUrl/ShortUrlService.php index 163989d8..95561fc5 100644 --- a/module/Core/src/ShortUrl/ShortUrlService.php +++ b/module/Core/src/ShortUrl/ShortUrlService.php @@ -34,7 +34,6 @@ class ShortUrlService implements ShortUrlServiceInterface ?ApiKey $apiKey = null, ): ShortUrl { if ($shortUrlEdit->longUrlWasProvided()) { - /** @var ShortUrlEdition $shortUrlEdit */ $shortUrlEdit = $this->titleResolutionHelper->processTitleAndValidateUrl($shortUrlEdit); } diff --git a/module/Core/src/ShortUrl/Transformer/ShortUrlDataTransformer.php b/module/Core/src/ShortUrl/Transformer/ShortUrlDataTransformer.php index 08327a98..9de5c408 100644 --- a/module/Core/src/ShortUrl/Transformer/ShortUrlDataTransformer.php +++ b/module/Core/src/ShortUrl/Transformer/ShortUrlDataTransformer.php @@ -27,6 +27,7 @@ class ShortUrlDataTransformer implements DataTransformerInterface 'shortCode' => $shortUrl->getShortCode(), 'shortUrl' => $this->stringifier->stringify($shortUrl), 'longUrl' => $shortUrl->getLongUrl(), + 'deviceLongUrls' => $shortUrl->deviceLongUrls(), 'dateCreated' => $shortUrl->getDateCreated()->toAtomString(), 'tags' => invoke($shortUrl->getTags(), '__toString'), 'meta' => $this->buildMeta($shortUrl), diff --git a/module/Core/src/ShortUrl/UrlShortener.php b/module/Core/src/ShortUrl/UrlShortener.php index d3f54650..7477052f 100644 --- a/module/Core/src/ShortUrl/UrlShortener.php +++ b/module/Core/src/ShortUrl/UrlShortener.php @@ -31,22 +31,21 @@ class UrlShortener implements UrlShortenerInterface * @throws NonUniqueSlugException * @throws InvalidUrlException */ - public function shorten(ShortUrlCreation $meta): ShortUrl + public function shorten(ShortUrlCreation $creation): ShortUrl { // First, check if a short URL exists for all provided params - $existingShortUrl = $this->findExistingShortUrlIfExists($meta); + $existingShortUrl = $this->findExistingShortUrlIfExists($creation); if ($existingShortUrl !== null) { return $existingShortUrl; } - /** @var \Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlCreation $meta */ - $meta = $this->titleResolutionHelper->processTitleAndValidateUrl($meta); + $creation = $this->titleResolutionHelper->processTitleAndValidateUrl($creation); /** @var ShortUrl $newShortUrl */ - $newShortUrl = $this->em->wrapInTransaction(function () use ($meta) { - $shortUrl = ShortUrl::create($meta, $this->relationResolver); + $newShortUrl = $this->em->wrapInTransaction(function () use ($creation): ShortUrl { + $shortUrl = ShortUrl::create($creation, $this->relationResolver); - $this->verifyShortCodeUniqueness($meta, $shortUrl); + $this->verifyShortCodeUniqueness($creation, $shortUrl); $this->em->persist($shortUrl); return $shortUrl; @@ -57,15 +56,15 @@ class UrlShortener implements UrlShortenerInterface return $newShortUrl; } - private function findExistingShortUrlIfExists(ShortUrlCreation $meta): ?ShortUrl + private function findExistingShortUrlIfExists(ShortUrlCreation $creation): ?ShortUrl { - if (! $meta->findIfExists()) { + if (! $creation->findIfExists) { return null; } /** @var ShortUrlRepositoryInterface $repo */ $repo = $this->em->getRepository(ShortUrl::class); - return $repo->findOneMatching($meta); + return $repo->findOneMatching($creation); } private function verifyShortCodeUniqueness(ShortUrlCreation $meta, ShortUrl $shortUrlToBeCreated): void @@ -77,7 +76,7 @@ class UrlShortener implements UrlShortenerInterface if (! $couldBeMadeUnique) { $domain = $shortUrlToBeCreated->getDomain(); - $domainAuthority = $domain?->getAuthority(); + $domainAuthority = $domain?->authority; throw NonUniqueSlugException::fromSlug($shortUrlToBeCreated->getShortCode(), $domainAuthority); } diff --git a/module/Core/src/ShortUrl/UrlShortenerInterface.php b/module/Core/src/ShortUrl/UrlShortenerInterface.php index c15b7ebf..d7ae45f0 100644 --- a/module/Core/src/ShortUrl/UrlShortenerInterface.php +++ b/module/Core/src/ShortUrl/UrlShortenerInterface.php @@ -15,5 +15,5 @@ interface UrlShortenerInterface * @throws NonUniqueSlugException * @throws InvalidUrlException */ - public function shorten(ShortUrlCreation $meta): ShortUrl; + public function shorten(ShortUrlCreation $creation): ShortUrl; } diff --git a/module/Core/src/Util/UrlValidator.php b/module/Core/src/Util/UrlValidator.php index 0057660a..8a7d0b89 100644 --- a/module/Core/src/Util/UrlValidator.php +++ b/module/Core/src/Util/UrlValidator.php @@ -26,7 +26,7 @@ class UrlValidator implements UrlValidatorInterface, RequestMethodInterface { private const MAX_REDIRECTS = 15; private const CHROME_USER_AGENT = 'Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) ' - . 'Chrome/51.0.2704.103 Safari/537.36'; + . 'Chrome/108.0.0.0 Safari/537.36'; public function __construct(private ClientInterface $httpClient, private UrlShortenerOptions $options) { diff --git a/module/Core/src/Visit/VisitsTracker.php b/module/Core/src/Visit/VisitsTracker.php index f97fc618..dd5fff91 100644 --- a/module/Core/src/Visit/VisitsTracker.php +++ b/module/Core/src/Visit/VisitsTracker.php @@ -15,9 +15,9 @@ use Shlinkio\Shlink\Core\Visit\Model\Visitor; class VisitsTracker implements VisitsTrackerInterface { public function __construct( - private ORM\EntityManagerInterface $em, - private EventDispatcherInterface $eventDispatcher, - private TrackingOptions $options, + private readonly ORM\EntityManagerInterface $em, + private readonly EventDispatcherInterface $eventDispatcher, + private readonly TrackingOptions $options, ) { } @@ -62,6 +62,9 @@ class VisitsTracker implements VisitsTrackerInterface $this->trackVisit($createVisit, $visitor); } + /** + * @param callable(Visitor $visitor): Visit $createVisit + */ private function trackVisit(callable $createVisit, Visitor $visitor): void { if ($this->options->disableTracking) { 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-db/Domain/Repository/DomainRepositoryTest.php b/module/Core/test-db/Domain/Repository/DomainRepositoryTest.php index c96d70ff..2b005947 100644 --- a/module/Core/test-db/Domain/Repository/DomainRepositoryTest.php +++ b/module/Core/test-db/Domain/Repository/DomainRepositoryTest.php @@ -131,7 +131,7 @@ class DomainRepositoryTest extends DatabaseTestCase { return ShortUrl::create( ShortUrlCreation::fromRawData( - ['domain' => $domain->getAuthority(), 'apiKey' => $apiKey, 'longUrl' => 'foo'], + ['domain' => $domain->authority, 'apiKey' => $apiKey, 'longUrl' => 'foo'], ), new class ($domain) implements ShortUrlRelationResolverInterface { public function __construct(private Domain $domain) diff --git a/module/Core/test-db/ShortUrl/Repository/ShortUrlRepositoryTest.php b/module/Core/test-db/ShortUrl/Repository/ShortUrlRepositoryTest.php index ed500349..0d90675a 100644 --- a/module/Core/test-db/ShortUrl/Repository/ShortUrlRepositoryTest.php +++ b/module/Core/test-db/ShortUrl/Repository/ShortUrlRepositoryTest.php @@ -134,7 +134,6 @@ class ShortUrlRepositoryTest extends DatabaseTestCase /** @test */ public function findOneMatchingReturnsNullForNonExistingShortUrls(): void { - self::assertNull($this->repo->findOneMatching(ShortUrlCreation::createEmpty())); self::assertNull($this->repo->findOneMatching(ShortUrlCreation::fromRawData(['longUrl' => 'foobar']))); self::assertNull($this->repo->findOneMatching( ShortUrlCreation::fromRawData(['longUrl' => 'foobar', 'tags' => ['foo', 'bar']]), @@ -270,7 +269,7 @@ class ShortUrlRepositoryTest extends DatabaseTestCase $shortUrl = ShortUrl::create(ShortUrlCreation::fromRawData([ 'validSince' => $start, 'apiKey' => $apiKey, - 'domain' => $rightDomain->getAuthority(), + 'domain' => $rightDomain->authority, 'longUrl' => 'foo', 'tags' => ['foo', 'bar'], ]), $this->relationResolver); @@ -313,7 +312,7 @@ class ShortUrlRepositoryTest extends DatabaseTestCase $shortUrl, $this->repo->findOneMatching(ShortUrlCreation::fromRawData([ 'validSince' => $start, - 'domain' => $rightDomain->getAuthority(), + 'domain' => $rightDomain->authority, 'longUrl' => 'foo', 'tags' => ['foo', 'bar'], ])), @@ -322,7 +321,7 @@ class ShortUrlRepositoryTest extends DatabaseTestCase $shortUrl, $this->repo->findOneMatching(ShortUrlCreation::fromRawData([ 'validSince' => $start, - 'domain' => $rightDomain->getAuthority(), + 'domain' => $rightDomain->authority, 'apiKey' => $rightDomainApiKey, 'longUrl' => 'foo', 'tags' => ['foo', 'bar'], @@ -332,7 +331,7 @@ class ShortUrlRepositoryTest extends DatabaseTestCase $shortUrl, $this->repo->findOneMatching(ShortUrlCreation::fromRawData([ 'validSince' => $start, - 'domain' => $rightDomain->getAuthority(), + 'domain' => $rightDomain->authority, 'apiKey' => $apiKey, 'longUrl' => 'foo', 'tags' => ['foo', 'bar'], @@ -341,7 +340,7 @@ class ShortUrlRepositoryTest extends DatabaseTestCase self::assertNull( $this->repo->findOneMatching(ShortUrlCreation::fromRawData([ 'validSince' => $start, - 'domain' => $rightDomain->getAuthority(), + 'domain' => $rightDomain->authority, 'apiKey' => $wrongDomainApiKey, 'longUrl' => 'foo', 'tags' => ['foo', 'bar'], diff --git a/module/Core/test-db/Tag/Repository/TagRepositoryTest.php b/module/Core/test-db/Tag/Repository/TagRepositoryTest.php index 57b3a795..97873b20 100644 --- a/module/Core/test-db/Tag/Repository/TagRepositoryTest.php +++ b/module/Core/test-db/Tag/Repository/TagRepositoryTest.php @@ -75,7 +75,7 @@ class TagRepositoryTest extends DatabaseTestCase [$firstUrlTags] = array_chunk($names, 3); $secondUrlTags = [$names[0]]; $metaWithTags = static fn (array $tags, ?ApiKey $apiKey) => ShortUrlCreation::fromRawData( - ['longUrl' => '', 'tags' => $tags, 'apiKey' => $apiKey], + ['longUrl' => 'longUrl', 'tags' => $tags, 'apiKey' => $apiKey], ); $shortUrl = ShortUrl::create($metaWithTags($firstUrlTags, $apiKey), $this->relationResolver); @@ -242,14 +242,14 @@ class TagRepositoryTest extends DatabaseTestCase [$firstUrlTags, $secondUrlTags] = array_chunk($names, 3); $shortUrl = ShortUrl::create( - ShortUrlCreation::fromRawData(['apiKey' => $authorApiKey, 'longUrl' => '', 'tags' => $firstUrlTags]), + ShortUrlCreation::fromRawData(['apiKey' => $authorApiKey, 'longUrl' => 'longUrl', 'tags' => $firstUrlTags]), $this->relationResolver, ); $this->getEntityManager()->persist($shortUrl); $shortUrl2 = ShortUrl::create( ShortUrlCreation::fromRawData( - ['domain' => $domain->getAuthority(), 'longUrl' => '', 'tags' => $secondUrlTags], + ['domain' => $domain->authority, 'longUrl' => 'longUrl', 'tags' => $secondUrlTags], ), $this->relationResolver, ); diff --git a/module/Core/test-db/Visit/Repository/VisitLocationRepositoryTest.php b/module/Core/test-db/Visit/Repository/VisitLocationRepositoryTest.php index 77f4c1e6..6b7c1e0d 100644 --- a/module/Core/test-db/Visit/Repository/VisitLocationRepositoryTest.php +++ b/module/Core/test-db/Visit/Repository/VisitLocationRepositoryTest.php @@ -31,7 +31,7 @@ class VisitLocationRepositoryTest extends DatabaseTestCase */ public function findVisitsReturnsProperVisits(int $blockSize): void { - $shortUrl = ShortUrl::createEmpty(); + $shortUrl = ShortUrl::createFake(); $this->getEntityManager()->persist($shortUrl); for ($i = 0; $i < 6; $i++) { diff --git a/module/Core/test-db/Visit/Repository/VisitRepositoryTest.php b/module/Core/test-db/Visit/Repository/VisitRepositoryTest.php index 2e509aa2..f1fed415 100644 --- a/module/Core/test-db/Visit/Repository/VisitRepositoryTest.php +++ b/module/Core/test-db/Visit/Repository/VisitRepositoryTest.php @@ -264,7 +264,9 @@ class VisitRepositoryTest extends DatabaseTestCase $apiKey1 = ApiKey::fromMeta(ApiKeyMeta::withRoles(RoleDefinition::forAuthoredShortUrls())); $this->getEntityManager()->persist($apiKey1); $shortUrl = ShortUrl::create( - ShortUrlCreation::fromRawData(['apiKey' => $apiKey1, 'domain' => $domain->getAuthority(), 'longUrl' => '']), + ShortUrlCreation::fromRawData( + ['apiKey' => $apiKey1, 'domain' => $domain->authority, 'longUrl' => 'longUrl'], + ), $this->relationResolver, ); $this->getEntityManager()->persist($shortUrl); @@ -272,12 +274,14 @@ class VisitRepositoryTest extends DatabaseTestCase $apiKey2 = ApiKey::fromMeta(ApiKeyMeta::withRoles(RoleDefinition::forAuthoredShortUrls())); $this->getEntityManager()->persist($apiKey2); - $shortUrl2 = ShortUrl::create(ShortUrlCreation::fromRawData(['apiKey' => $apiKey2, 'longUrl' => ''])); + $shortUrl2 = ShortUrl::create(ShortUrlCreation::fromRawData(['apiKey' => $apiKey2, 'longUrl' => 'longUrl'])); $this->getEntityManager()->persist($shortUrl2); $this->createVisitsForShortUrl($shortUrl2, 5); $shortUrl3 = ShortUrl::create( - ShortUrlCreation::fromRawData(['apiKey' => $apiKey2, 'domain' => $domain->getAuthority(), 'longUrl' => '']), + ShortUrlCreation::fromRawData( + ['apiKey' => $apiKey2, 'domain' => $domain->authority, 'longUrl' => 'longUrl'], + ), $this->relationResolver, ); $this->getEntityManager()->persist($shortUrl3); @@ -315,7 +319,7 @@ class VisitRepositoryTest extends DatabaseTestCase /** @test */ public function findOrphanVisitsReturnsExpectedResult(): void { - $shortUrl = ShortUrl::create(ShortUrlCreation::fromRawData(['longUrl' => ''])); + $shortUrl = ShortUrl::create(ShortUrlCreation::fromRawData(['longUrl' => 'longUrl'])); $this->getEntityManager()->persist($shortUrl); $this->createVisitsForShortUrl($shortUrl, 7); @@ -364,7 +368,7 @@ class VisitRepositoryTest extends DatabaseTestCase /** @test */ public function countOrphanVisitsReturnsExpectedResult(): void { - $shortUrl = ShortUrl::create(ShortUrlCreation::fromRawData(['longUrl' => ''])); + $shortUrl = ShortUrl::create(ShortUrlCreation::fromRawData(['longUrl' => 'longUrl'])); $this->getEntityManager()->persist($shortUrl); $this->createVisitsForShortUrl($shortUrl, 7); @@ -460,7 +464,7 @@ class VisitRepositoryTest extends DatabaseTestCase } /** - * @return array{string, string, \Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl} + * @return array{string, string, ShortUrl} */ private function createShortUrlsAndVisits( bool|string $withDomain = true, @@ -468,7 +472,7 @@ class VisitRepositoryTest extends DatabaseTestCase ?ApiKey $apiKey = null, ): array { $shortUrl = ShortUrl::create(ShortUrlCreation::fromRawData([ - ShortUrlInputFilter::LONG_URL => '', + ShortUrlInputFilter::LONG_URL => 'longUrl', ShortUrlInputFilter::TAGS => $tags, ShortUrlInputFilter::API_KEY => $apiKey, ]), $this->relationResolver); @@ -482,7 +486,7 @@ class VisitRepositoryTest extends DatabaseTestCase $shortUrlWithDomain = ShortUrl::create(ShortUrlCreation::fromRawData([ 'customSlug' => $shortCode, 'domain' => $domain, - 'longUrl' => '', + 'longUrl' => 'longUrl', ])); $this->getEntityManager()->persist($shortUrlWithDomain); $this->createVisitsForShortUrl($shortUrlWithDomain, 3); diff --git a/module/Core/test/Action/QrCodeActionTest.php b/module/Core/test/Action/QrCodeActionTest.php index 599a09d9..684e9217 100644 --- a/module/Core/test/Action/QrCodeActionTest.php +++ b/module/Core/test/Action/QrCodeActionTest.php @@ -56,7 +56,7 @@ class QrCodeActionTest extends TestCase $shortCode = 'abc123'; $this->urlResolver->expects($this->once())->method('resolveEnabledShortUrl')->with( ShortUrlIdentifier::fromShortCodeAndDomain($shortCode, ''), - )->willReturn(ShortUrl::createEmpty()); + )->willReturn(ShortUrl::createFake()); $delegate = $this->createMock(RequestHandlerInterface::class); $delegate->expects($this->never())->method('handle'); @@ -78,7 +78,7 @@ class QrCodeActionTest extends TestCase $code = 'abc123'; $this->urlResolver->method('resolveEnabledShortUrl')->with( ShortUrlIdentifier::fromShortCodeAndDomain($code, ''), - )->willReturn(ShortUrl::createEmpty()); + )->willReturn(ShortUrl::createFake()); $delegate = $this->createMock(RequestHandlerInterface::class); $req = (new ServerRequest())->withAttribute('shortCode', $code)->withQueryParams($query); @@ -111,7 +111,7 @@ class QrCodeActionTest extends TestCase $code = 'abc123'; $this->urlResolver->method('resolveEnabledShortUrl')->with( ShortUrlIdentifier::fromShortCodeAndDomain($code, ''), - )->willReturn(ShortUrl::createEmpty()); + )->willReturn(ShortUrl::createFake()); $delegate = $this->createMock(RequestHandlerInterface::class); $resp = $this->action($defaultOptions)->process($req->withAttribute('shortCode', $code), $delegate); diff --git a/module/Core/test/Config/EnvVarsTest.php b/module/Core/test/Config/EnvVarsTest.php index ff4878de..6d4b1394 100644 --- a/module/Core/test/Config/EnvVarsTest.php +++ b/module/Core/test/Config/EnvVarsTest.php @@ -7,7 +7,6 @@ namespace ShlinkioTest\Shlink\Core\Config; use PHPUnit\Framework\TestCase; use Shlinkio\Shlink\Core\Config\EnvVars; -use function Functional\map; use function putenv; class EnvVarsTest extends TestCase @@ -59,11 +58,4 @@ class EnvVarsTest extends TestCase yield 'DB_DRIVER without default' => [EnvVars::DB_DRIVER, null, null]; yield 'DB_DRIVER with default' => [EnvVars::DB_DRIVER, 'foobar', 'foobar']; } - - /** @test */ - public function allValuesCanBeListed(): void - { - $expected = map(EnvVars::cases(), static fn (EnvVars $envVar) => $envVar->value); - self::assertEquals(EnvVars::values(), $expected); - } } diff --git a/module/Core/test/EventDispatcher/LocateVisitTest.php b/module/Core/test/EventDispatcher/LocateVisitTest.php index cad6d164..d538bff0 100644 --- a/module/Core/test/EventDispatcher/LocateVisitTest.php +++ b/module/Core/test/EventDispatcher/LocateVisitTest.php @@ -70,7 +70,7 @@ class LocateVisitTest extends TestCase { $event = new UrlVisited('123'); $this->em->expects($this->once())->method('find')->with(Visit::class, '123')->willReturn( - Visit::forValidShortUrl(ShortUrl::createEmpty(), new Visitor('', '', '1.2.3.4', '')), + Visit::forValidShortUrl(ShortUrl::createFake(), new Visitor('', '', '1.2.3.4', '')), ); $this->em->expects($this->never())->method('flush'); $this->dbUpdater->expects($this->once())->method('databaseFileExists')->withAnyParameters()->willReturn(false); @@ -89,7 +89,7 @@ class LocateVisitTest extends TestCase { $event = new UrlVisited('123'); $this->em->expects($this->once())->method('find')->with(Visit::class, '123')->willReturn( - Visit::forValidShortUrl(ShortUrl::createEmpty(), new Visitor('', '', '1.2.3.4', '')), + Visit::forValidShortUrl(ShortUrl::createFake(), new Visitor('', '', '1.2.3.4', '')), ); $this->em->expects($this->never())->method('flush'); $this->dbUpdater->expects($this->once())->method('databaseFileExists')->withAnyParameters()->willReturn(true); @@ -110,7 +110,7 @@ class LocateVisitTest extends TestCase { $event = new UrlVisited('123'); $this->em->expects($this->once())->method('find')->with(Visit::class, '123')->willReturn( - Visit::forValidShortUrl(ShortUrl::createEmpty(), new Visitor('', '', '1.2.3.4', '')), + Visit::forValidShortUrl(ShortUrl::createFake(), new Visitor('', '', '1.2.3.4', '')), ); $this->em->expects($this->never())->method('flush'); $this->dbUpdater->expects($this->once())->method('databaseFileExists')->withAnyParameters()->willReturn(true); @@ -148,7 +148,7 @@ class LocateVisitTest extends TestCase public function provideNonLocatableVisits(): iterable { - $shortUrl = ShortUrl::createEmpty(); + $shortUrl = ShortUrl::createFake(); yield 'null IP' => [Visit::forValidShortUrl($shortUrl, new Visitor('', '', null, ''))]; yield 'empty IP' => [Visit::forValidShortUrl($shortUrl, new Visitor('', '', '', ''))]; @@ -183,11 +183,11 @@ class LocateVisitTest extends TestCase public function provideIpAddresses(): iterable { yield 'no original IP address' => [ - Visit::forValidShortUrl(ShortUrl::createEmpty(), new Visitor('', '', '1.2.3.4', '')), + Visit::forValidShortUrl(ShortUrl::createFake(), new Visitor('', '', '1.2.3.4', '')), null, ]; yield 'original IP address' => [ - Visit::forValidShortUrl(ShortUrl::createEmpty(), new Visitor('', '', '1.2.3.4', '')), + Visit::forValidShortUrl(ShortUrl::createFake(), new Visitor('', '', '1.2.3.4', '')), '1.2.3.4', ]; yield 'base url' => [Visit::forBasePath(new Visitor('', '', '1.2.3.4', '')), '1.2.3.4']; diff --git a/module/Core/test/EventDispatcher/Mercure/NotifyNewShortUrlToMercureTest.php b/module/Core/test/EventDispatcher/Mercure/NotifyNewShortUrlToMercureTest.php index c42bd915..855f6c14 100644 --- a/module/Core/test/EventDispatcher/Mercure/NotifyNewShortUrlToMercureTest.php +++ b/module/Core/test/EventDispatcher/Mercure/NotifyNewShortUrlToMercureTest.php @@ -57,7 +57,7 @@ class NotifyNewShortUrlToMercureTest extends TestCase /** @test */ public function expectedNotificationIsPublished(): void { - $shortUrl = ShortUrl::withLongUrl(''); + $shortUrl = ShortUrl::withLongUrl('longUrl'); $update = Update::forTopicAndPayload('', []); $this->em->expects($this->once())->method('find')->with(ShortUrl::class, '123')->willReturn($shortUrl); @@ -74,7 +74,7 @@ class NotifyNewShortUrlToMercureTest extends TestCase /** @test */ public function messageIsPrintedIfPublishingFails(): void { - $shortUrl = ShortUrl::withLongUrl(''); + $shortUrl = ShortUrl::withLongUrl('longUrl'); $update = Update::forTopicAndPayload('', []); $e = new Exception('Error'); diff --git a/module/Core/test/EventDispatcher/Mercure/NotifyVisitToMercureTest.php b/module/Core/test/EventDispatcher/Mercure/NotifyVisitToMercureTest.php index 1cecada7..23450fd3 100644 --- a/module/Core/test/EventDispatcher/Mercure/NotifyVisitToMercureTest.php +++ b/module/Core/test/EventDispatcher/Mercure/NotifyVisitToMercureTest.php @@ -59,7 +59,7 @@ class NotifyVisitToMercureTest extends TestCase public function notificationsAreSentWhenVisitIsFound(): void { $visitId = '123'; - $visit = Visit::forValidShortUrl(ShortUrl::createEmpty(), Visitor::emptyInstance()); + $visit = Visit::forValidShortUrl(ShortUrl::createFake(), Visitor::emptyInstance()); $update = Update::forTopicAndPayload('', []); $this->em->expects($this->once())->method('find')->with(Visit::class, $visitId)->willReturn($visit); @@ -79,7 +79,7 @@ class NotifyVisitToMercureTest extends TestCase public function debugIsLoggedWhenExceptionIsThrown(): void { $visitId = '123'; - $visit = Visit::forValidShortUrl(ShortUrl::createEmpty(), Visitor::emptyInstance()); + $visit = Visit::forValidShortUrl(ShortUrl::createFake(), Visitor::emptyInstance()); $update = Update::forTopicAndPayload('', []); $e = new RuntimeException('Error'); diff --git a/module/Core/test/EventDispatcher/NotifyVisitToWebHooksTest.php b/module/Core/test/EventDispatcher/NotifyVisitToWebHooksTest.php index 7a5cb888..17b26a74 100644 --- a/module/Core/test/EventDispatcher/NotifyVisitToWebHooksTest.php +++ b/module/Core/test/EventDispatcher/NotifyVisitToWebHooksTest.php @@ -123,7 +123,7 @@ class NotifyVisitToWebHooksTest extends TestCase public function provideVisits(): iterable { yield 'regular visit' => [ - Visit::forValidShortUrl(ShortUrl::createEmpty(), Visitor::emptyInstance()), + Visit::forValidShortUrl(ShortUrl::createFake(), Visitor::emptyInstance()), ['shortUrl', 'visit'], ]; yield 'orphan visit' => [Visit::forBasePath(Visitor::emptyInstance()), ['visit'],]; diff --git a/module/Core/test/EventDispatcher/PublishingUpdatesGeneratorTest.php b/module/Core/test/EventDispatcher/PublishingUpdatesGeneratorTest.php index cda8fe98..924996f9 100644 --- a/module/Core/test/EventDispatcher/PublishingUpdatesGeneratorTest.php +++ b/module/Core/test/EventDispatcher/PublishingUpdatesGeneratorTest.php @@ -38,7 +38,7 @@ class PublishingUpdatesGeneratorTest extends TestCase { $shortUrl = ShortUrl::create(ShortUrlCreation::fromRawData([ 'customSlug' => 'foo', - 'longUrl' => '', + 'longUrl' => 'longUrl', 'title' => $title, ])); $visit = Visit::forValidShortUrl($shortUrl, Visitor::emptyInstance()); @@ -51,7 +51,8 @@ class PublishingUpdatesGeneratorTest extends TestCase 'shortUrl' => [ 'shortCode' => $shortUrl->getShortCode(), 'shortUrl' => 'http:/' . $shortUrl->getShortCode(), - 'longUrl' => '', + 'longUrl' => 'longUrl', + 'deviceLongUrls' => $shortUrl->deviceLongUrls(), 'dateCreated' => $shortUrl->getDateCreated()->toAtomString(), 'visitsCount' => 0, 'tags' => [], @@ -118,7 +119,7 @@ class PublishingUpdatesGeneratorTest extends TestCase { $shortUrl = ShortUrl::create(ShortUrlCreation::fromRawData([ 'customSlug' => 'foo', - 'longUrl' => '', + 'longUrl' => 'longUrl', 'title' => 'The title', ])); @@ -128,7 +129,8 @@ class PublishingUpdatesGeneratorTest extends TestCase self::assertEquals(['shortUrl' => [ 'shortCode' => $shortUrl->getShortCode(), 'shortUrl' => 'http:/' . $shortUrl->getShortCode(), - 'longUrl' => '', + 'longUrl' => 'longUrl', + 'deviceLongUrls' => $shortUrl->deviceLongUrls(), 'dateCreated' => $shortUrl->getDateCreated()->toAtomString(), 'visitsCount' => 0, 'tags' => [], diff --git a/module/Core/test/EventDispatcher/RabbitMq/NotifyNewShortUrlToRabbitMqTest.php b/module/Core/test/EventDispatcher/RabbitMq/NotifyNewShortUrlToRabbitMqTest.php index 764f7949..52e9630d 100644 --- a/module/Core/test/EventDispatcher/RabbitMq/NotifyNewShortUrlToRabbitMqTest.php +++ b/module/Core/test/EventDispatcher/RabbitMq/NotifyNewShortUrlToRabbitMqTest.php @@ -68,7 +68,7 @@ class NotifyNewShortUrlToRabbitMqTest extends TestCase $shortUrlId = '123'; $update = Update::forTopicAndPayload(Topic::NEW_SHORT_URL->value, []); $this->em->expects($this->once())->method('find')->with(ShortUrl::class, $shortUrlId)->willReturn( - ShortUrl::withLongUrl(''), + ShortUrl::withLongUrl('longUrl'), ); $this->updatesGenerator->expects($this->once())->method('newShortUrlUpdate')->with( $this->isInstanceOf(ShortUrl::class), @@ -88,7 +88,7 @@ class NotifyNewShortUrlToRabbitMqTest extends TestCase $shortUrlId = '123'; $update = Update::forTopicAndPayload(Topic::NEW_SHORT_URL->value, []); $this->em->expects($this->once())->method('find')->with(ShortUrl::class, $shortUrlId)->willReturn( - ShortUrl::withLongUrl(''), + ShortUrl::withLongUrl('longUrl'), ); $this->updatesGenerator->expects($this->once())->method('newShortUrlUpdate')->with( $this->isInstanceOf(ShortUrl::class), diff --git a/module/Core/test/EventDispatcher/RabbitMq/NotifyVisitToRabbitMqTest.php b/module/Core/test/EventDispatcher/RabbitMq/NotifyVisitToRabbitMqTest.php index 8b7b392c..6211ad2b 100644 --- a/module/Core/test/EventDispatcher/RabbitMq/NotifyVisitToRabbitMqTest.php +++ b/module/Core/test/EventDispatcher/RabbitMq/NotifyVisitToRabbitMqTest.php @@ -159,7 +159,7 @@ class NotifyVisitToRabbitMqTest extends TestCase { yield 'legacy non-orphan visit' => [ true, - $visit = Visit::forValidShortUrl(ShortUrl::withLongUrl(''), Visitor::emptyInstance()), + $visit = Visit::forValidShortUrl(ShortUrl::withLongUrl('longUrl'), Visitor::emptyInstance()), noop(...), function (MockObject & PublishingHelperInterface $helper) use ($visit): void { $helper->method('publishUpdate')->with($this->callback(function (Update $update) use ($visit): bool { @@ -190,7 +190,7 @@ class NotifyVisitToRabbitMqTest extends TestCase ]; yield 'non-legacy non-orphan visit' => [ false, - Visit::forValidShortUrl(ShortUrl::withLongUrl(''), Visitor::emptyInstance()), + Visit::forValidShortUrl(ShortUrl::withLongUrl('longUrl'), Visitor::emptyInstance()), function (MockObject & PublishingUpdatesGeneratorInterface $updatesGenerator): void { $update = Update::forTopicAndPayload('', []); $updatesGenerator->expects($this->never())->method('newOrphanVisitUpdate'); diff --git a/module/Core/test/EventDispatcher/RedisPubSub/NotifyNewShortUrlToRedisTest.php b/module/Core/test/EventDispatcher/RedisPubSub/NotifyNewShortUrlToRedisTest.php index 0b5dfd27..a913de15 100644 --- a/module/Core/test/EventDispatcher/RedisPubSub/NotifyNewShortUrlToRedisTest.php +++ b/module/Core/test/EventDispatcher/RedisPubSub/NotifyNewShortUrlToRedisTest.php @@ -55,7 +55,7 @@ class NotifyNewShortUrlToRedisTest extends TestCase $shortUrlId = '123'; $update = Update::forTopicAndPayload(Topic::NEW_SHORT_URL->value, []); $this->em->expects($this->once())->method('find')->with(ShortUrl::class, $shortUrlId)->willReturn( - ShortUrl::withLongUrl(''), + ShortUrl::withLongUrl('longUrl'), ); $this->updatesGenerator->expects($this->once())->method('newShortUrlUpdate')->with( $this->isInstanceOf(ShortUrl::class), diff --git a/module/Core/test/Functions/FunctionsTest.php b/module/Core/test/Functions/FunctionsTest.php new file mode 100644 index 00000000..ad45812f --- /dev/null +++ b/module/Core/test/Functions/FunctionsTest.php @@ -0,0 +1,45 @@ + $enum + * @test + * @dataProvider provideEnums + */ + public function enumValuesReturnsExpectedValueForEnum(string $enum, array $expectedValues): void + { + self::assertEquals($expectedValues, enumValues($enum)); + } + + public function provideEnums(): iterable + { + yield EnvVars::class => [EnvVars::class, map(EnvVars::cases(), static fn (EnvVars $envVar) => $envVar->value)]; + yield VisitType::class => [ + VisitType::class, + map(VisitType::cases(), static fn (VisitType $envVar) => $envVar->value), + ]; + yield DeviceType::class => [ + DeviceType::class, + map(DeviceType::cases(), static fn (DeviceType $envVar) => $envVar->value), + ]; + yield OrderableField::class => [ + OrderableField::class, + map(OrderableField::cases(), static fn (OrderableField $envVar) => $envVar->value), + ]; + } +} diff --git a/module/Core/test/Importer/ImportedLinksProcessorTest.php b/module/Core/test/Importer/ImportedLinksProcessorTest.php index c480e11a..f1b2f3bb 100644 --- a/module/Core/test/Importer/ImportedLinksProcessorTest.php +++ b/module/Core/test/Importer/ImportedLinksProcessorTest.php @@ -210,7 +210,7 @@ class ImportedLinksProcessorTest extends TestCase ]), 'Skipped. Imported 4 visits', 4, - ShortUrl::createEmpty(), + ShortUrl::createFake(), ]; yield 'existing short URL with previous imported visits' => [ $createImportedUrl([ @@ -222,8 +222,8 @@ class ImportedLinksProcessorTest extends TestCase ]), 'Skipped. Imported 2 visits', 2, - ShortUrl::createEmpty()->setVisits(new ArrayCollection([ - Visit::fromImport(ShortUrl::createEmpty(), new ImportedShlinkVisit('', '', $now, null)), + ShortUrl::createFake()->setVisits(new ArrayCollection([ + Visit::fromImport(ShortUrl::createFake(), new ImportedShlinkVisit('', '', $now, null)), ])), ]; } diff --git a/module/Core/test/ShortUrl/DeleteShortUrlServiceTest.php b/module/Core/test/ShortUrl/DeleteShortUrlServiceTest.php index be036264..3173e2ee 100644 --- a/module/Core/test/ShortUrl/DeleteShortUrlServiceTest.php +++ b/module/Core/test/ShortUrl/DeleteShortUrlServiceTest.php @@ -29,8 +29,8 @@ class DeleteShortUrlServiceTest extends TestCase protected function setUp(): void { - $shortUrl = ShortUrl::createEmpty()->setVisits(new ArrayCollection( - map(range(0, 10), fn () => Visit::forValidShortUrl(ShortUrl::createEmpty(), Visitor::emptyInstance())), + $shortUrl = ShortUrl::createFake()->setVisits(new ArrayCollection( + map(range(0, 10), fn () => Visit::forValidShortUrl(ShortUrl::createFake(), Visitor::emptyInstance())), )); $this->shortCode = $shortUrl->getShortCode(); diff --git a/module/Core/test/ShortUrl/Entity/ShortUrlTest.php b/module/Core/test/ShortUrl/Entity/ShortUrlTest.php index 026778ae..2d950d5f 100644 --- a/module/Core/test/ShortUrl/Entity/ShortUrlTest.php +++ b/module/Core/test/ShortUrl/Entity/ShortUrlTest.php @@ -7,8 +7,10 @@ namespace ShlinkioTest\Shlink\Core\ShortUrl\Entity; use Cake\Chronos\Chronos; use PHPUnit\Framework\TestCase; use Shlinkio\Shlink\Core\Exception\ShortCodeCannotBeRegeneratedException; +use Shlinkio\Shlink\Core\Model\DeviceType; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlCreation; +use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlEdition; use Shlinkio\Shlink\Core\ShortUrl\Model\Validation\ShortUrlInputFilter; use Shlinkio\Shlink\Importer\Model\ImportedShlinkUrl; use Shlinkio\Shlink\Importer\Sources\ImportSource; @@ -38,11 +40,11 @@ class ShortUrlTest extends TestCase public function provideInvalidShortUrls(): iterable { yield 'with custom slug' => [ - ShortUrl::create(ShortUrlCreation::fromRawData(['customSlug' => 'custom-slug', 'longUrl' => ''])), + ShortUrl::create(ShortUrlCreation::fromRawData(['customSlug' => 'custom-slug', 'longUrl' => 'longUrl'])), 'The short code cannot be regenerated on ShortUrls where a custom slug was provided.', ]; yield 'already persisted' => [ - ShortUrl::createEmpty()->setId('1'), + ShortUrl::createFake()->setId('1'), 'The short code can be regenerated only on new ShortUrls which have not been persisted yet.', ]; } @@ -64,9 +66,9 @@ class ShortUrlTest extends TestCase public function provideValidShortUrls(): iterable { - yield 'no custom slug' => [ShortUrl::createEmpty()]; + yield 'no custom slug' => [ShortUrl::createFake()]; yield 'imported with custom slug' => [ShortUrl::fromImport( - new ImportedShlinkUrl(ImportSource::BITLY, '', [], Chronos::now(), null, 'custom-slug', null), + new ImportedShlinkUrl(ImportSource::BITLY, 'longUrl', [], Chronos::now(), null, 'custom-slug', null), true, )]; } @@ -78,7 +80,7 @@ class ShortUrlTest extends TestCase public function shortCodesHaveExpectedLength(?int $length, int $expectedLength): void { $shortUrl = ShortUrl::create(ShortUrlCreation::fromRawData( - [ShortUrlInputFilter::SHORT_CODE_LENGTH => $length, 'longUrl' => ''], + [ShortUrlInputFilter::SHORT_CODE_LENGTH => $length, 'longUrl' => 'longUrl'], )); self::assertEquals($expectedLength, strlen($shortUrl->getShortCode())); @@ -89,4 +91,46 @@ class ShortUrlTest extends TestCase yield [null, DEFAULT_SHORT_CODES_LENGTH]; yield from map(range(4, 10), fn (int $value) => [$value, $value]); } + + /** @test */ + public function deviceLongUrlsAreUpdated(): void + { + $shortUrl = ShortUrl::withLongUrl('foo'); + + $shortUrl->update(ShortUrlEdition::fromRawData([ + ShortUrlInputFilter::DEVICE_LONG_URLS => [ + DeviceType::ANDROID->value => 'android', + DeviceType::IOS->value => 'ios', + ], + ])); + self::assertEquals([ + DeviceType::ANDROID->value => 'android', + DeviceType::IOS->value => 'ios', + DeviceType::DESKTOP->value => null, + ], $shortUrl->deviceLongUrls()); + + $shortUrl->update(ShortUrlEdition::fromRawData([ + ShortUrlInputFilter::DEVICE_LONG_URLS => [ + DeviceType::ANDROID->value => null, + DeviceType::DESKTOP->value => 'desktop', + ], + ])); + self::assertEquals([ + DeviceType::ANDROID->value => null, + DeviceType::IOS->value => 'ios', + DeviceType::DESKTOP->value => 'desktop', + ], $shortUrl->deviceLongUrls()); + + $shortUrl->update(ShortUrlEdition::fromRawData([ + ShortUrlInputFilter::DEVICE_LONG_URLS => [ + DeviceType::ANDROID->value => null, + DeviceType::IOS->value => null, + ], + ])); + self::assertEquals([ + DeviceType::ANDROID->value => null, + DeviceType::IOS->value => null, + DeviceType::DESKTOP->value => 'desktop', + ], $shortUrl->deviceLongUrls()); + } } diff --git a/module/Core/test/ShortUrl/Helper/ShortUrlRedirectionBuilderTest.php b/module/Core/test/ShortUrl/Helper/ShortUrlRedirectionBuilderTest.php index cb94a9f1..341ff6bf 100644 --- a/module/Core/test/ShortUrl/Helper/ShortUrlRedirectionBuilderTest.php +++ b/module/Core/test/ShortUrl/Helper/ShortUrlRedirectionBuilderTest.php @@ -4,12 +4,19 @@ declare(strict_types=1); 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; @@ -26,74 +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, $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, + ]; } } diff --git a/module/Core/test/ShortUrl/Helper/ShortUrlStringifierTest.php b/module/Core/test/ShortUrl/Helper/ShortUrlStringifierTest.php index b6d5a123..fc8c7579 100644 --- a/module/Core/test/ShortUrl/Helper/ShortUrlStringifierTest.php +++ b/module/Core/test/ShortUrl/Helper/ShortUrlStringifierTest.php @@ -30,7 +30,7 @@ class ShortUrlStringifierTest extends TestCase { $shortUrlWithShortCode = fn (string $shortCode, ?string $domain = null) => ShortUrl::create( ShortUrlCreation::fromRawData([ - 'longUrl' => '', + 'longUrl' => 'longUrl', 'customSlug' => $shortCode, 'domain' => $domain, ]), diff --git a/module/Core/test/ShortUrl/Middleware/ExtraPathRedirectMiddlewareTest.php b/module/Core/test/ShortUrl/Middleware/ExtraPathRedirectMiddlewareTest.php index 835d1487..c157403e 100644 --- a/module/Core/test/ShortUrl/Middleware/ExtraPathRedirectMiddlewareTest.php +++ b/module/Core/test/ShortUrl/Middleware/ExtraPathRedirectMiddlewareTest.php @@ -142,7 +142,7 @@ class ExtraPathRedirectMiddlewareTest extends TestCase $type->method('isInvalidShortUrl')->willReturn(true); $request = ServerRequestFactory::fromGlobals()->withAttribute(NotFoundType::class, $type) ->withUri(new Uri('https://s.test/shortCode/bar/baz')); - $shortUrl = ShortUrl::withLongUrl(''); + $shortUrl = ShortUrl::withLongUrl('longUrl'); $currentIteration = 1; $this->resolver->expects($this->exactly($expectedResolveCalls))->method('resolveEnabledShortUrl')->with( @@ -159,7 +159,7 @@ class ExtraPathRedirectMiddlewareTest extends TestCase ); $this->redirectionBuilder->expects($this->once())->method('buildShortUrlRedirect')->with( $shortUrl, - [], + $this->isInstanceOf(ServerRequestInterface::class), $expectedExtraPath, )->willReturn('the_built_long_url'); $this->redirectResponseHelper->expects($this->once())->method('buildRedirectResponse')->with( diff --git a/module/Core/test/ShortUrl/Model/ShortUrlCreationTest.php b/module/Core/test/ShortUrl/Model/ShortUrlCreationTest.php index ee9e540a..4d11289c 100644 --- a/module/Core/test/ShortUrl/Model/ShortUrlCreationTest.php +++ b/module/Core/test/ShortUrl/Model/ShortUrlCreationTest.php @@ -8,6 +8,7 @@ use Cake\Chronos\Chronos; use PHPUnit\Framework\TestCase; use Shlinkio\Shlink\Core\Config\EnvVars; use Shlinkio\Shlink\Core\Exception\ValidationException; +use Shlinkio\Shlink\Core\Model\DeviceType; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlCreation; use Shlinkio\Shlink\Core\ShortUrl\Model\Validation\ShortUrlInputFilter; use stdClass; @@ -69,6 +70,40 @@ class ShortUrlCreationTest extends TestCase yield [[ ShortUrlInputFilter::LONG_URL => [], ]]; + yield [[ + ShortUrlInputFilter::LONG_URL => null, + ]]; + yield [[ + ShortUrlInputFilter::LONG_URL => 'foo', + ShortUrlInputFilter::DEVICE_LONG_URLS => [ + 'invalid' => 'https://shlink.io', + ], + ]]; + yield [[ + ShortUrlInputFilter::LONG_URL => 'foo', + ShortUrlInputFilter::DEVICE_LONG_URLS => [ + DeviceType::DESKTOP->value => '', + ], + ]]; + yield [[ + ShortUrlInputFilter::LONG_URL => 'foo', + ShortUrlInputFilter::DEVICE_LONG_URLS => [ + DeviceType::DESKTOP->value => null, + ], + ]]; + yield [[ + ShortUrlInputFilter::LONG_URL => 'foo', + ShortUrlInputFilter::DEVICE_LONG_URLS => [ + DeviceType::IOS->value => ' ', + ], + ]]; + yield [[ + ShortUrlInputFilter::LONG_URL => 'foo', + ShortUrlInputFilter::DEVICE_LONG_URLS => [ + DeviceType::IOS->value => 'bar', + DeviceType::ANDROID->value => [], + ], + ]]; } /** @@ -80,24 +115,24 @@ class ShortUrlCreationTest extends TestCase string $expectedSlug, bool $multiSegmentEnabled = false, ): void { - $meta = ShortUrlCreation::fromRawData([ + $creation = ShortUrlCreation::fromRawData([ 'validSince' => Chronos::parse('2015-01-01')->toAtomString(), 'customSlug' => $customSlug, - 'longUrl' => '', + 'longUrl' => 'longUrl', EnvVars::MULTI_SEGMENT_SLUGS_ENABLED->value => $multiSegmentEnabled, ]); - self::assertTrue($meta->hasValidSince()); - self::assertEquals(Chronos::parse('2015-01-01'), $meta->getValidSince()); + self::assertTrue($creation->hasValidSince()); + self::assertEquals(Chronos::parse('2015-01-01'), $creation->validSince); - self::assertFalse($meta->hasValidUntil()); - self::assertNull($meta->getValidUntil()); + self::assertFalse($creation->hasValidUntil()); + self::assertNull($creation->validUntil); - self::assertTrue($meta->hasCustomSlug()); - self::assertEquals($expectedSlug, $meta->getCustomSlug()); + self::assertTrue($creation->hasCustomSlug()); + self::assertEquals($expectedSlug, $creation->customSlug); - self::assertFalse($meta->hasMaxVisits()); - self::assertNull($meta->getMaxVisits()); + self::assertFalse($creation->hasMaxVisits()); + self::assertNull($creation->maxVisits); } public function provideCustomSlugs(): iterable @@ -127,12 +162,12 @@ class ShortUrlCreationTest extends TestCase */ public function titleIsCroppedIfTooLong(?string $title, ?string $expectedTitle): void { - $meta = ShortUrlCreation::fromRawData([ + $creation = ShortUrlCreation::fromRawData([ 'title' => $title, - 'longUrl' => '', + 'longUrl' => 'longUrl', ]); - self::assertEquals($expectedTitle, $meta->getTitle()); + self::assertEquals($expectedTitle, $creation->title); } public function provideTitles(): iterable @@ -153,12 +188,12 @@ class ShortUrlCreationTest extends TestCase */ public function emptyDomainIsDiscarded(?string $domain, ?string $expectedDomain): void { - $meta = ShortUrlCreation::fromRawData([ + $creation = ShortUrlCreation::fromRawData([ 'domain' => $domain, - 'longUrl' => '', + 'longUrl' => 'longUrl', ]); - self::assertSame($expectedDomain, $meta->getDomain()); + self::assertSame($expectedDomain, $creation->domain); } public function provideDomains(): iterable diff --git a/module/Core/test/ShortUrl/Model/ShortUrlEditionTest.php b/module/Core/test/ShortUrl/Model/ShortUrlEditionTest.php new file mode 100644 index 00000000..e03bb1ac --- /dev/null +++ b/module/Core/test/ShortUrl/Model/ShortUrlEditionTest.php @@ -0,0 +1,54 @@ + $deviceLongUrls]); + + self::assertEquals($expectedDeviceLongUrls, $edition->deviceLongUrls); + self::assertEquals($expectedDevicesToRemove, $edition->devicesToRemove); + } + + public function provideDeviceLongUrls(): iterable + { + yield 'null' => [null, [], []]; + yield 'empty' => [[], [], []]; + yield 'only new urls' => [[ + DeviceType::DESKTOP->value => 'foo', + DeviceType::IOS->value => 'bar', + ], [ + DeviceType::DESKTOP->value => DeviceLongUrlPair::fromRawTypeAndLongUrl(DeviceType::DESKTOP->value, 'foo'), + DeviceType::IOS->value => DeviceLongUrlPair::fromRawTypeAndLongUrl(DeviceType::IOS->value, 'bar'), + ], []]; + yield 'only urls to remove' => [[ + DeviceType::ANDROID->value => null, + DeviceType::IOS->value => null, + ], [], [DeviceType::ANDROID, DeviceType::IOS]]; + yield 'both' => [[ + DeviceType::DESKTOP->value => 'bar', + DeviceType::IOS->value => 'foo', + DeviceType::ANDROID->value => null, + ], [ + DeviceType::DESKTOP->value => DeviceLongUrlPair::fromRawTypeAndLongUrl(DeviceType::DESKTOP->value, 'bar'), + DeviceType::IOS->value => DeviceLongUrlPair::fromRawTypeAndLongUrl(DeviceType::IOS->value, 'foo'), + ], [DeviceType::ANDROID]]; + } +} diff --git a/module/Core/test/ShortUrl/Model/Validation/DeviceLongUrlsValidatorTest.php b/module/Core/test/ShortUrl/Model/Validation/DeviceLongUrlsValidatorTest.php new file mode 100644 index 00000000..8bac2f98 --- /dev/null +++ b/module/Core/test/ShortUrl/Model/Validation/DeviceLongUrlsValidatorTest.php @@ -0,0 +1,71 @@ +validator = new DeviceLongUrlsValidator(new NotEmpty()); + } + + /** + * @test + * @dataProvider provideNonArrayValues + */ + public function nonArrayValuesAreNotValid(mixed $invalidValue): void + { + self::assertFalse($this->validator->isValid($invalidValue)); + self::assertEquals(['NOT_ARRAY' => 'Provided value is not an array.'], $this->validator->getMessages()); + } + + public function provideNonArrayValues(): iterable + { + yield 'int' => [0]; + yield 'float' => [100.45]; + yield 'string' => ['foo']; + yield 'boolean' => [true]; + yield 'object' => [new stdClass()]; + yield 'null' => [null]; + } + + /** @test */ + public function unrecognizedKeysAreNotValid(): void + { + self::assertFalse($this->validator->isValid(['foo' => 'bar'])); + self::assertEquals( + ['INVALID_DEVICE' => 'You have provided at least one invalid device identifier.'], + $this->validator->getMessages(), + ); + } + + /** @test */ + public function everyUrlMustMatchLongUrlValidator(): void + { + self::assertFalse($this->validator->isValid([DeviceType::ANDROID->value => ''])); + self::assertEquals( + ['INVALID_LONG_URL' => 'At least one of the long URLs are invalid.'], + $this->validator->getMessages(), + ); + } + + /** @test */ + public function validValuesResultInValidResult(): void + { + self::assertTrue($this->validator->isValid([ + DeviceType::ANDROID->value => 'foo', + DeviceType::IOS->value => 'bar', + DeviceType::DESKTOP->value => 'baz', + ])); + } +} diff --git a/module/Core/test/ShortUrl/Resolver/PersistenceShortUrlRelationResolverTest.php b/module/Core/test/ShortUrl/Resolver/PersistenceShortUrlRelationResolverTest.php index 8734b95c..fedfd96f 100644 --- a/module/Core/test/ShortUrl/Resolver/PersistenceShortUrlRelationResolverTest.php +++ b/module/Core/test/ShortUrl/Resolver/PersistenceShortUrlRelationResolverTest.php @@ -52,7 +52,7 @@ class PersistenceShortUrlRelationResolverTest extends TestCase self::assertSame($result, $foundDomain); } self::assertInstanceOf(Domain::class, $result); - self::assertEquals($authority, $result->getAuthority()); + self::assertEquals($authority, $result->authority); } public function provideFoundDomains(): iterable diff --git a/module/Core/test/ShortUrl/Resolver/SimpleShortUrlRelationResolverTest.php b/module/Core/test/ShortUrl/Resolver/SimpleShortUrlRelationResolverTest.php index f0cc7023..443710bb 100644 --- a/module/Core/test/ShortUrl/Resolver/SimpleShortUrlRelationResolverTest.php +++ b/module/Core/test/ShortUrl/Resolver/SimpleShortUrlRelationResolverTest.php @@ -30,7 +30,7 @@ class SimpleShortUrlRelationResolverTest extends TestCase self::assertNull($result); } else { self::assertInstanceOf(Domain::class, $result); - self::assertEquals($domain, $result->getAuthority()); + self::assertEquals($domain, $result->authority); } } diff --git a/module/Core/test/ShortUrl/ShortUrlListServiceTest.php b/module/Core/test/ShortUrl/ShortUrlListServiceTest.php index be8eb852..446e95eb 100644 --- a/module/Core/test/ShortUrl/ShortUrlListServiceTest.php +++ b/module/Core/test/ShortUrl/ShortUrlListServiceTest.php @@ -36,10 +36,10 @@ class ShortUrlListServiceTest extends TestCase public function listedUrlsAreReturnedFromEntityManager(?ApiKey $apiKey): void { $list = [ - ShortUrl::createEmpty(), - ShortUrl::createEmpty(), - ShortUrl::createEmpty(), - ShortUrl::createEmpty(), + ShortUrl::createFake(), + ShortUrl::createFake(), + ShortUrl::createFake(), + ShortUrl::createFake(), ]; $this->repo->expects($this->once())->method('findList')->willReturn($list); diff --git a/module/Core/test/ShortUrl/ShortUrlResolverTest.php b/module/Core/test/ShortUrl/ShortUrlResolverTest.php index 9c2bcab3..177e432e 100644 --- a/module/Core/test/ShortUrl/ShortUrlResolverTest.php +++ b/module/Core/test/ShortUrl/ShortUrlResolverTest.php @@ -114,7 +114,7 @@ class ShortUrlResolverTest extends TestCase $now = Chronos::now(); yield 'maxVisits reached' => [(function () { - $shortUrl = ShortUrl::create(ShortUrlCreation::fromRawData(['maxVisits' => 3, 'longUrl' => ''])); + $shortUrl = ShortUrl::create(ShortUrlCreation::fromRawData(['maxVisits' => 3, 'longUrl' => 'longUrl'])); $shortUrl->setVisits(new ArrayCollection(map( range(0, 4), fn () => Visit::forValidShortUrl($shortUrl, Visitor::emptyInstance()), @@ -123,16 +123,16 @@ class ShortUrlResolverTest extends TestCase return $shortUrl; })()]; yield 'future validSince' => [ShortUrl::create(ShortUrlCreation::fromRawData( - ['validSince' => $now->addMonth()->toAtomString(), 'longUrl' => ''], + ['validSince' => $now->addMonth()->toAtomString(), 'longUrl' => 'longUrl'], ))]; yield 'past validUntil' => [ShortUrl::create(ShortUrlCreation::fromRawData( - ['validUntil' => $now->subMonth()->toAtomString(), 'longUrl' => ''], + ['validUntil' => $now->subMonth()->toAtomString(), 'longUrl' => 'longUrl'], ))]; yield 'mixed' => [(function () use ($now) { $shortUrl = ShortUrl::create(ShortUrlCreation::fromRawData([ 'maxVisits' => 3, 'validUntil' => $now->subMonth()->toAtomString(), - 'longUrl' => '', + 'longUrl' => 'longUrl', ])); $shortUrl->setVisits(new ArrayCollection(map( range(0, 4), diff --git a/module/Core/test/ShortUrl/ShortUrlServiceTest.php b/module/Core/test/ShortUrl/ShortUrlServiceTest.php index 9cc0d955..7851aa9b 100644 --- a/module/Core/test/ShortUrl/ShortUrlServiceTest.php +++ b/module/Core/test/ShortUrl/ShortUrlServiceTest.php @@ -7,7 +7,9 @@ namespace ShlinkioTest\Shlink\Core\ShortUrl; use Cake\Chronos\Chronos; use Doctrine\ORM\EntityManagerInterface; use PHPUnit\Framework\MockObject\MockObject; +use PHPUnit\Framework\MockObject\Rule\InvocationOrder; use PHPUnit\Framework\TestCase; +use Shlinkio\Shlink\Core\Model\DeviceType; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlTitleResolutionHelperInterface; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlEdition; @@ -18,26 +20,28 @@ use Shlinkio\Shlink\Core\ShortUrl\ShortUrlService; use Shlinkio\Shlink\Rest\Entity\ApiKey; use ShlinkioTest\Shlink\Core\Util\ApiKeyHelpersTrait; +use function array_fill_keys; +use function Shlinkio\Shlink\Core\enumValues; + class ShortUrlServiceTest extends TestCase { use ApiKeyHelpersTrait; private ShortUrlService $service; - private MockObject & EntityManagerInterface $em; private MockObject & ShortUrlResolverInterface $urlResolver; private MockObject & ShortUrlTitleResolutionHelperInterface $titleResolutionHelper; protected function setUp(): void { - $this->em = $this->createMock(EntityManagerInterface::class); - $this->em->method('persist')->willReturn(null); - $this->em->method('flush')->willReturn(null); + $em = $this->createMock(EntityManagerInterface::class); + $em->method('persist')->willReturn(null); + $em->method('flush')->willReturn(null); $this->urlResolver = $this->createMock(ShortUrlResolverInterface::class); $this->titleResolutionHelper = $this->createMock(ShortUrlTitleResolutionHelperInterface::class); $this->service = new ShortUrlService( - $this->em, + $em, $this->urlResolver, $this->titleResolutionHelper, new SimpleShortUrlRelationResolver(), @@ -49,7 +53,7 @@ class ShortUrlServiceTest extends TestCase * @dataProvider provideShortUrlEdits */ public function updateShortUrlUpdatesProvidedData( - int $expectedValidateCalls, + InvocationOrder $expectedValidateCalls, ShortUrlEdition $shortUrlEdit, ?ApiKey $apiKey, ): void { @@ -61,7 +65,7 @@ class ShortUrlServiceTest extends TestCase $apiKey, )->willReturn($shortUrl); - $this->titleResolutionHelper->expects($this->exactly($expectedValidateCalls)) + $this->titleResolutionHelper->expects($expectedValidateCalls) ->method('processTitleAndValidateUrl') ->with($shortUrlEdit) ->willReturn($shortUrlEdit); @@ -72,34 +76,44 @@ class ShortUrlServiceTest extends TestCase $apiKey, ); + $resolveDeviceLongUrls = function () use ($shortUrlEdit): array { + $result = array_fill_keys(enumValues(DeviceType::class), null); + foreach ($shortUrlEdit->deviceLongUrls ?? [] as $longUrl) { + $result[$longUrl->deviceType->value] = $longUrl->longUrl; + } + + return $result; + }; + self::assertSame($shortUrl, $result); - self::assertEquals($shortUrlEdit->validSince(), $shortUrl->getValidSince()); - self::assertEquals($shortUrlEdit->validUntil(), $shortUrl->getValidUntil()); - self::assertEquals($shortUrlEdit->maxVisits(), $shortUrl->getMaxVisits()); - self::assertEquals($shortUrlEdit->longUrl() ?? $originalLongUrl, $shortUrl->getLongUrl()); + self::assertEquals($shortUrlEdit->validSince, $shortUrl->getValidSince()); + self::assertEquals($shortUrlEdit->validUntil, $shortUrl->getValidUntil()); + self::assertEquals($shortUrlEdit->maxVisits, $shortUrl->getMaxVisits()); + self::assertEquals($shortUrlEdit->longUrl ?? $originalLongUrl, $shortUrl->getLongUrl()); + self::assertEquals($resolveDeviceLongUrls(), $shortUrl->deviceLongUrls()); } public function provideShortUrlEdits(): iterable { - yield 'no long URL' => [0, ShortUrlEdition::fromRawData( - [ - 'validSince' => Chronos::parse('2017-01-01 00:00:00')->toAtomString(), - 'validUntil' => Chronos::parse('2017-01-05 00:00:00')->toAtomString(), - 'maxVisits' => 5, + yield 'no long URL' => [$this->never(), ShortUrlEdition::fromRawData([ + 'validSince' => Chronos::parse('2017-01-01 00:00:00')->toAtomString(), + 'validUntil' => Chronos::parse('2017-01-05 00:00:00')->toAtomString(), + 'maxVisits' => 5, + ]), null]; + yield 'long URL and API key' => [$this->once(), ShortUrlEdition::fromRawData([ + 'validSince' => Chronos::parse('2017-01-01 00:00:00')->toAtomString(), + 'maxVisits' => 10, + 'longUrl' => 'modifiedLongUrl', + ]), ApiKey::create()]; + yield 'long URL with validation' => [$this->once(), ShortUrlEdition::fromRawData([ + 'longUrl' => 'modifiedLongUrl', + 'validateUrl' => true, + ]), null]; + yield 'device redirects' => [$this->never(), ShortUrlEdition::fromRawData([ + 'deviceLongUrls' => [ + DeviceType::IOS->value => 'iosLongUrl', + DeviceType::ANDROID->value => 'androidLongUrl', ], - ), null]; - yield 'long URL' => [1, ShortUrlEdition::fromRawData( - [ - 'validSince' => Chronos::parse('2017-01-01 00:00:00')->toAtomString(), - 'maxVisits' => 10, - 'longUrl' => 'modifiedLongUrl', - ], - ), ApiKey::create()]; - yield 'long URL with validation' => [1, ShortUrlEdition::fromRawData( - [ - 'longUrl' => 'modifiedLongUrl', - 'validateUrl' => true, - ], - ), null]; + ]), null]; } } diff --git a/module/Core/test/ShortUrl/Transformer/ShortUrlDataTransformerTest.php b/module/Core/test/ShortUrl/Transformer/ShortUrlDataTransformerTest.php index c9df4e38..6159294b 100644 --- a/module/Core/test/ShortUrl/Transformer/ShortUrlDataTransformerTest.php +++ b/module/Core/test/ShortUrl/Transformer/ShortUrlDataTransformerTest.php @@ -38,14 +38,14 @@ class ShortUrlDataTransformerTest extends TestCase $maxVisits = random_int(1, 1000); $now = Chronos::now(); - yield 'no metadata' => [ShortUrl::createEmpty(), [ + yield 'no metadata' => [ShortUrl::createFake(), [ 'validSince' => null, 'validUntil' => null, 'maxVisits' => null, ]]; yield 'max visits only' => [ShortUrl::create(ShortUrlCreation::fromRawData([ 'maxVisits' => $maxVisits, - 'longUrl' => '', + 'longUrl' => 'longUrl', ])), [ 'validSince' => null, 'validUntil' => null, @@ -53,7 +53,7 @@ class ShortUrlDataTransformerTest extends TestCase ]]; yield 'max visits and valid since' => [ ShortUrl::create(ShortUrlCreation::fromRawData( - ['validSince' => $now, 'maxVisits' => $maxVisits, 'longUrl' => ''], + ['validSince' => $now, 'maxVisits' => $maxVisits, 'longUrl' => 'longUrl'], )), [ 'validSince' => $now->toAtomString(), @@ -63,7 +63,7 @@ class ShortUrlDataTransformerTest extends TestCase ]; yield 'both dates' => [ ShortUrl::create(ShortUrlCreation::fromRawData( - ['validSince' => $now, 'validUntil' => $now->subDays(10), 'longUrl' => ''], + ['validSince' => $now, 'validUntil' => $now->subDays(10), 'longUrl' => 'longUrl'], )), [ 'validSince' => $now->toAtomString(), @@ -72,9 +72,12 @@ class ShortUrlDataTransformerTest extends TestCase ], ]; yield 'everything' => [ - ShortUrl::create(ShortUrlCreation::fromRawData( - ['validSince' => $now, 'validUntil' => $now->subDays(5), 'maxVisits' => $maxVisits, 'longUrl' => ''], - )), + ShortUrl::create(ShortUrlCreation::fromRawData([ + 'validSince' => $now, + 'validUntil' => $now->subDays(5), + 'maxVisits' => $maxVisits, + 'longUrl' => 'longUrl', + ])), [ 'validSince' => $now->toAtomString(), 'validUntil' => $now->subDays(5)->toAtomString(), diff --git a/module/Core/test/Visit/Entity/VisitTest.php b/module/Core/test/Visit/Entity/VisitTest.php index 7024c946..5ae22005 100644 --- a/module/Core/test/Visit/Entity/VisitTest.php +++ b/module/Core/test/Visit/Entity/VisitTest.php @@ -18,7 +18,7 @@ class VisitTest extends TestCase */ public function isProperlyJsonSerialized(string $userAgent, bool $expectedToBePotentialBot): void { - $visit = Visit::forValidShortUrl(ShortUrl::createEmpty(), new Visitor($userAgent, 'some site', '1.2.3.4', '')); + $visit = Visit::forValidShortUrl(ShortUrl::createFake(), new Visitor($userAgent, 'some site', '1.2.3.4', '')); self::assertEquals([ 'referer' => 'some site', @@ -48,7 +48,7 @@ class VisitTest extends TestCase public function addressIsAnonymizedWhenRequested(bool $anonymize, ?string $address, ?string $expectedAddress): void { $visit = Visit::forValidShortUrl( - ShortUrl::createEmpty(), + ShortUrl::createFake(), new Visitor('Chrome', 'some site', $address, ''), $anonymize, ); diff --git a/module/Core/test/Visit/VisitsStatsHelperTest.php b/module/Core/test/Visit/VisitsStatsHelperTest.php index 1774ba6a..0ed06e7d 100644 --- a/module/Core/test/Visit/VisitsStatsHelperTest.php +++ b/module/Core/test/Visit/VisitsStatsHelperTest.php @@ -86,7 +86,7 @@ class VisitsStatsHelperTest extends TestCase $repo = $this->createMock(ShortUrlRepositoryInterface::class); $repo->expects($this->once())->method('shortCodeIsInUse')->with($identifier, $spec)->willReturn(true); - $list = map(range(0, 1), fn () => Visit::forValidShortUrl(ShortUrl::createEmpty(), Visitor::emptyInstance())); + $list = map(range(0, 1), fn () => Visit::forValidShortUrl(ShortUrl::createFake(), Visitor::emptyInstance())); $repo2 = $this->createMock(VisitRepository::class); $repo2->method('findVisitsByShortCode')->with( $identifier, @@ -146,7 +146,7 @@ class VisitsStatsHelperTest extends TestCase $repo = $this->createMock(TagRepository::class); $repo->expects($this->once())->method('tagExists')->with($tag, $apiKey)->willReturn(true); - $list = map(range(0, 1), fn () => Visit::forValidShortUrl(ShortUrl::createEmpty(), Visitor::emptyInstance())); + $list = map(range(0, 1), fn () => Visit::forValidShortUrl(ShortUrl::createFake(), Visitor::emptyInstance())); $repo2 = $this->createMock(VisitRepository::class); $repo2->method('findVisitsByTag')->with($tag, $this->isInstanceOf(VisitsListFiltering::class))->willReturn( $list, @@ -187,7 +187,7 @@ class VisitsStatsHelperTest extends TestCase $repo = $this->createMock(DomainRepository::class); $repo->expects($this->once())->method('domainExists')->with($domain, $apiKey)->willReturn(true); - $list = map(range(0, 1), fn () => Visit::forValidShortUrl(ShortUrl::createEmpty(), Visitor::emptyInstance())); + $list = map(range(0, 1), fn () => Visit::forValidShortUrl(ShortUrl::createFake(), Visitor::emptyInstance())); $repo2 = $this->createMock(VisitRepository::class); $repo2->method('findVisitsByDomain')->with( $domain, @@ -217,7 +217,7 @@ class VisitsStatsHelperTest extends TestCase $repo = $this->createMock(DomainRepository::class); $repo->expects($this->never())->method('domainExists'); - $list = map(range(0, 1), fn () => Visit::forValidShortUrl(ShortUrl::createEmpty(), Visitor::emptyInstance())); + $list = map(range(0, 1), fn () => Visit::forValidShortUrl(ShortUrl::createFake(), Visitor::emptyInstance())); $repo2 = $this->createMock(VisitRepository::class); $repo2->method('findVisitsByDomain')->with( 'DEFAULT', @@ -259,7 +259,7 @@ class VisitsStatsHelperTest extends TestCase /** @test */ public function nonOrphanVisitsAreReturnedAsExpected(): void { - $list = map(range(0, 3), fn () => Visit::forValidShortUrl(ShortUrl::createEmpty(), Visitor::emptyInstance())); + $list = map(range(0, 3), fn () => Visit::forValidShortUrl(ShortUrl::createFake(), Visitor::emptyInstance())); $repo = $this->createMock(VisitRepository::class); $repo->expects($this->once())->method('countNonOrphanVisits')->with( $this->isInstanceOf(VisitsCountFiltering::class), diff --git a/module/Core/test/Visit/VisitsTrackerTest.php b/module/Core/test/Visit/VisitsTrackerTest.php index d981f755..9c27d5df 100644 --- a/module/Core/test/Visit/VisitsTrackerTest.php +++ b/module/Core/test/Visit/VisitsTrackerTest.php @@ -58,7 +58,7 @@ class VisitsTrackerTest extends TestCase public function provideTrackingMethodNames(): iterable { - yield 'track' => ['track', [ShortUrl::createEmpty(), Visitor::emptyInstance()]]; + yield 'track' => ['track', [ShortUrl::createFake(), Visitor::emptyInstance()]]; yield 'trackInvalidShortUrlVisit' => ['trackInvalidShortUrlVisit', [Visitor::emptyInstance()]]; yield 'trackBaseUrlVisit' => ['trackBaseUrlVisit', [Visitor::emptyInstance()]]; yield 'trackRegularNotFoundVisit' => ['trackRegularNotFoundVisit', [Visitor::emptyInstance()]]; diff --git a/module/Rest/src/ApiKey/Model/RoleDefinition.php b/module/Rest/src/ApiKey/Model/RoleDefinition.php index 6132772a..403e6214 100644 --- a/module/Rest/src/ApiKey/Model/RoleDefinition.php +++ b/module/Rest/src/ApiKey/Model/RoleDefinition.php @@ -22,7 +22,7 @@ final class RoleDefinition { return new self( Role::DOMAIN_SPECIFIC, - ['domain_id' => $domain->getId(), 'authority' => $domain->getAuthority()], + ['domain_id' => $domain->getId(), 'authority' => $domain->authority], ); } } diff --git a/module/Rest/src/Entity/ApiKey.php b/module/Rest/src/Entity/ApiKey.php index beb9e0f9..57fecdd0 100644 --- a/module/Rest/src/Entity/ApiKey.php +++ b/module/Rest/src/Entity/ApiKey.php @@ -21,7 +21,7 @@ class ApiKey extends AbstractEntity private string $key; private ?Chronos $expirationDate = null; private bool $enabled; - /** @var Collection|ApiKeyRole[] */ + /** @var Collection */ private Collection $roles; private ?string $name = null; @@ -147,12 +147,9 @@ class ApiKey extends AbstractEntity $meta = $roleDefinition->meta; if ($this->hasRole($role)) { - /** @var ApiKeyRole $apiKeyRole */ - $apiKeyRole = $this->roles->get($role->value); - $apiKeyRole->updateMeta($meta); + $this->roles->get($role->value)?->updateMeta($meta); } else { - $apiKeyRole = new ApiKeyRole($roleDefinition->role, $roleDefinition->meta, $this); - $this->roles[$role->value] = $apiKeyRole; + $this->roles->set($role->value, new ApiKeyRole($role, $meta, $this)); } } } diff --git a/module/Rest/src/Middleware/ShortUrl/OverrideDomainMiddleware.php b/module/Rest/src/Middleware/ShortUrl/OverrideDomainMiddleware.php index f4d01e97..8a88e340 100644 --- a/module/Rest/src/Middleware/ShortUrl/OverrideDomainMiddleware.php +++ b/module/Rest/src/Middleware/ShortUrl/OverrideDomainMiddleware.php @@ -34,11 +34,11 @@ class OverrideDomainMiddleware implements MiddlewareInterface if ($requestMethod === RequestMethodInterface::METHOD_POST) { /** @var array $payload */ $payload = $request->getParsedBody(); - $payload[ShortUrlInputFilter::DOMAIN] = $domain->getAuthority(); + $payload[ShortUrlInputFilter::DOMAIN] = $domain->authority; return $handler->handle($request->withParsedBody($payload)); } - return $handler->handle($request->withAttribute(ShortUrlInputFilter::DOMAIN, $domain->getAuthority())); + return $handler->handle($request->withAttribute(ShortUrlInputFilter::DOMAIN, $domain->authority)); } } diff --git a/module/Rest/test-api/Action/CreateShortUrlTest.php b/module/Rest/test-api/Action/CreateShortUrlTest.php index 5190000e..0bb02c9e 100644 --- a/module/Rest/test-api/Action/CreateShortUrlTest.php +++ b/module/Rest/test-api/Action/CreateShortUrlTest.php @@ -118,7 +118,7 @@ class CreateShortUrlTest extends ApiTestCase public function provideMaxVisits(): array { - return map(range(10, 15), fn (int $i) => [$i]); + return map(range(10, 15), fn(int $i) => [$i]); } /** @test */ @@ -172,12 +172,14 @@ class CreateShortUrlTest extends ApiTestCase yield 'only long URL' => [['longUrl' => $longUrl]]; yield 'long URL and tags' => [['longUrl' => $longUrl, 'tags' => ['boo', 'far']]]; yield 'long URL and custom slug' => [['longUrl' => $longUrl, 'customSlug' => 'my cool slug']]; - yield 'several params' => [[ - 'longUrl' => $longUrl, - 'tags' => ['boo', 'far'], - 'validSince' => Chronos::now()->toAtomString(), - 'maxVisits' => 7, - ]]; + yield 'several params' => [ + [ + 'longUrl' => $longUrl, + 'tags' => ['boo', 'far'], + 'validSince' => Chronos::now()->toAtomString(), + 'maxVisits' => 7, + ], + ]; } /** @@ -261,21 +263,20 @@ class CreateShortUrlTest extends ApiTestCase public function provideInvalidUrls(): iterable { - yield 'empty URL' => ['', '2', 'INVALID_URL']; - yield 'non-reachable URL' => ['https://this-has-to-be-invalid.com', '2', 'INVALID_URL']; - yield 'API version 3' => ['', '3', 'https://shlink.io/api/error/invalid-url']; + yield 'API version 2' => ['https://this-has-to-be-invalid.com', '2', 'INVALID_URL']; + yield 'API version 3' => ['https://this-has-to-be-invalid.com', '3', 'https://shlink.io/api/error/invalid-url']; } /** * @test * @dataProvider provideInvalidArgumentApiVersions */ - public function failsToCreateShortUrlWithoutLongUrl(string $version, string $expectedType): void + public function failsToCreateShortUrlWithoutLongUrl(array $payload, string $version, string $expectedType): void { $resp = $this->callApiWithKey( self::METHOD_POST, sprintf('/rest/v%s/short-urls', $version), - [RequestOptions::JSON => []], + [RequestOptions::JSON => $payload], ); $payload = $this->getJsonResponsePayload($resp); @@ -288,8 +289,22 @@ class CreateShortUrlTest extends ApiTestCase public function provideInvalidArgumentApiVersions(): iterable { - yield ['2', 'INVALID_ARGUMENT']; - yield ['3', 'https://shlink.io/api/error/invalid-data']; + yield 'missing long url v2' => [[], '2', 'INVALID_ARGUMENT']; + yield 'missing long url v3' => [[], '3', 'https://shlink.io/api/error/invalid-data']; + yield 'empty long url v2' => [['longUrl' => null], '2', 'INVALID_ARGUMENT']; + yield 'empty long url v3' => [['longUrl' => ' '], '3', 'https://shlink.io/api/error/invalid-data']; + yield 'empty device long url v2' => [[ + 'longUrl' => 'foo', + 'deviceLongUrls' => [ + 'android' => null, + ], + ], '2', 'INVALID_ARGUMENT']; + yield 'empty device long url v3' => [[ + 'longUrl' => 'foo', + 'deviceLongUrls' => [ + 'ios' => ' ', + ], + ], '3', 'https://shlink.io/api/error/invalid-data']; } /** @test */ @@ -362,6 +377,22 @@ class CreateShortUrlTest extends ApiTestCase self::assertEquals('http://s.test/🦣🦣🦣', $payload['shortUrl']); } + /** @test */ + public function canCreateShortUrlsWithDeviceLongUrls(): void + { + [$statusCode, $payload] = $this->createShortUrl([ + 'longUrl' => 'https://github.com/shlinkio/shlink/issues/1557', + 'deviceLongUrls' => [ + 'ios' => 'https://github.com/shlinkio/shlink/ios', + 'android' => 'https://github.com/shlinkio/shlink/android', + ], + ]); + + self::assertEquals(self::STATUS_OK, $statusCode); + self::assertEquals('https://github.com/shlinkio/shlink/ios', $payload['deviceLongUrls']['ios'] ?? null); + self::assertEquals('https://github.com/shlinkio/shlink/android', $payload['deviceLongUrls']['android'] ?? null); + } + /** * @return array{int, array} */ diff --git a/module/Rest/test-api/Action/EditShortUrlTest.php b/module/Rest/test-api/Action/EditShortUrlTest.php index fefbdcba..74fdebc5 100644 --- a/module/Rest/test-api/Action/EditShortUrlTest.php +++ b/module/Rest/test-api/Action/EditShortUrlTest.php @@ -154,7 +154,7 @@ class EditShortUrlTest extends ApiTestCase $editResp = $this->callApiWithKey(self::METHOD_PATCH, (string) $url, [RequestOptions::JSON => [ 'maxVisits' => 100, ]]); - $editedShortUrl = $this->getJsonResponsePayload($this->callApiWithKey(self::METHOD_GET, (string) $url)); + $editedShortUrl = $this->getJsonResponsePayload($editResp); self::assertEquals(self::STATUS_OK, $editResp->getStatusCode()); self::assertEquals($domain, $editedShortUrl['domain']); @@ -170,4 +170,27 @@ class EditShortUrlTest extends ApiTestCase ]; yield 'no domain' => [null, 'https://shlink.io/documentation/']; } + + /** @test */ + public function deviceLongUrlsCanBeEdited(): void + { + $shortCode = 'def456'; + $url = new Uri(sprintf('/short-urls/%s', $shortCode)); + $editResp = $this->callApiWithKey(self::METHOD_PATCH, (string) $url, [RequestOptions::JSON => [ + 'deviceLongUrls' => [ + 'android' => null, // This one will get removed + 'ios' => 'https://blog.alejandrocelaya.com/ios/edited', // This one will be edited + 'desktop' => 'https://blog.alejandrocelaya.com/desktop', // This one is new and will be created + ], + ]]); + $deviceLongUrls = $this->getJsonResponsePayload($editResp)['deviceLongUrls'] ?? []; + + self::assertEquals(self::STATUS_OK, $editResp->getStatusCode()); + self::assertArrayHasKey('ios', $deviceLongUrls); + self::assertEquals('https://blog.alejandrocelaya.com/ios/edited', $deviceLongUrls['ios']); + self::assertArrayHasKey('desktop', $deviceLongUrls); + self::assertEquals('https://blog.alejandrocelaya.com/desktop', $deviceLongUrls['desktop']); + self::assertArrayHasKey('android', $deviceLongUrls); + self::assertNull($deviceLongUrls['android']); + } } diff --git a/module/Rest/test-api/Action/ListShortUrlsTest.php b/module/Rest/test-api/Action/ListShortUrlsTest.php index 43d466e3..5cddfc50 100644 --- a/module/Rest/test-api/Action/ListShortUrlsTest.php +++ b/module/Rest/test-api/Action/ListShortUrlsTest.php @@ -6,6 +6,7 @@ namespace ShlinkioApiTest\Shlink\Rest\Action; use Cake\Chronos\Chronos; use GuzzleHttp\RequestOptions; +use Shlinkio\Shlink\Core\Model\DeviceType; use Shlinkio\Shlink\TestUtils\ApiTest\ApiTestCase; use function count; @@ -169,109 +170,124 @@ class ListShortUrlsTest extends ApiTestCase public function provideFilteredLists(): iterable { + // FIXME Cannot use enums in constants in PHP 8.1. Change this once support for PHP 8.1 is dropped + $withDeviceLongUrls = static fn (array $shortUrl, ?array $longUrls = null) => [ + ...$shortUrl, + 'deviceLongUrls' => $longUrls ?? [ + DeviceType::ANDROID->value => null, + DeviceType::IOS->value => null, + DeviceType::DESKTOP->value => null, + ], + ]; + $shortUrlMeta = $withDeviceLongUrls(self::SHORT_URL_META, [ + DeviceType::ANDROID->value => 'https://blog.alejandrocelaya.com/android', + DeviceType::IOS->value => 'https://blog.alejandrocelaya.com/ios', + DeviceType::DESKTOP->value => null, + ]); + yield [[], [ - self::SHORT_URL_CUSTOM_DOMAIN, - self::SHORT_URL_CUSTOM_SLUG, - self::SHORT_URL_META, - self::SHORT_URL_CUSTOM_SLUG_AND_DOMAIN, - self::SHORT_URL_SHLINK_WITH_TITLE, - self::SHORT_URL_DOCS, + $withDeviceLongUrls(self::SHORT_URL_CUSTOM_DOMAIN), + $withDeviceLongUrls(self::SHORT_URL_CUSTOM_SLUG), + $shortUrlMeta, + $withDeviceLongUrls(self::SHORT_URL_CUSTOM_SLUG_AND_DOMAIN), + $withDeviceLongUrls(self::SHORT_URL_SHLINK_WITH_TITLE), + $withDeviceLongUrls(self::SHORT_URL_DOCS), ], 'valid_api_key']; yield [['excludePastValidUntil' => 'true'], [ - self::SHORT_URL_CUSTOM_DOMAIN, - self::SHORT_URL_CUSTOM_SLUG, - self::SHORT_URL_META, - self::SHORT_URL_CUSTOM_SLUG_AND_DOMAIN, - self::SHORT_URL_SHLINK_WITH_TITLE, + $withDeviceLongUrls(self::SHORT_URL_CUSTOM_DOMAIN), + $withDeviceLongUrls(self::SHORT_URL_CUSTOM_SLUG), + $shortUrlMeta, + $withDeviceLongUrls(self::SHORT_URL_CUSTOM_SLUG_AND_DOMAIN), + $withDeviceLongUrls(self::SHORT_URL_SHLINK_WITH_TITLE), ], 'valid_api_key']; yield [['excludeMaxVisitsReached' => 'true'], [ - self::SHORT_URL_CUSTOM_DOMAIN, - self::SHORT_URL_CUSTOM_SLUG, - self::SHORT_URL_META, - self::SHORT_URL_CUSTOM_SLUG_AND_DOMAIN, - self::SHORT_URL_DOCS, + $withDeviceLongUrls(self::SHORT_URL_CUSTOM_DOMAIN), + $withDeviceLongUrls(self::SHORT_URL_CUSTOM_SLUG), + $shortUrlMeta, + $withDeviceLongUrls(self::SHORT_URL_CUSTOM_SLUG_AND_DOMAIN), + $withDeviceLongUrls(self::SHORT_URL_DOCS), ], 'valid_api_key']; yield [['orderBy' => 'shortCode'], [ - self::SHORT_URL_SHLINK_WITH_TITLE, - self::SHORT_URL_CUSTOM_SLUG, - self::SHORT_URL_CUSTOM_SLUG_AND_DOMAIN, - self::SHORT_URL_META, - self::SHORT_URL_DOCS, - self::SHORT_URL_CUSTOM_DOMAIN, + $withDeviceLongUrls(self::SHORT_URL_SHLINK_WITH_TITLE), + $withDeviceLongUrls(self::SHORT_URL_CUSTOM_SLUG), + $withDeviceLongUrls(self::SHORT_URL_CUSTOM_SLUG_AND_DOMAIN), + $shortUrlMeta, + $withDeviceLongUrls(self::SHORT_URL_DOCS), + $withDeviceLongUrls(self::SHORT_URL_CUSTOM_DOMAIN), ], 'valid_api_key']; yield [['orderBy' => 'shortCode-DESC'], [ - self::SHORT_URL_DOCS, - self::SHORT_URL_CUSTOM_DOMAIN, - self::SHORT_URL_META, - self::SHORT_URL_CUSTOM_SLUG_AND_DOMAIN, - self::SHORT_URL_CUSTOM_SLUG, - self::SHORT_URL_SHLINK_WITH_TITLE, + $withDeviceLongUrls(self::SHORT_URL_DOCS), + $withDeviceLongUrls(self::SHORT_URL_CUSTOM_DOMAIN), + $shortUrlMeta, + $withDeviceLongUrls(self::SHORT_URL_CUSTOM_SLUG_AND_DOMAIN), + $withDeviceLongUrls(self::SHORT_URL_CUSTOM_SLUG), + $withDeviceLongUrls(self::SHORT_URL_SHLINK_WITH_TITLE), ], 'valid_api_key']; yield [['orderBy' => 'title-DESC'], [ - self::SHORT_URL_META, - self::SHORT_URL_CUSTOM_SLUG, - self::SHORT_URL_DOCS, - self::SHORT_URL_CUSTOM_DOMAIN, - self::SHORT_URL_CUSTOM_SLUG_AND_DOMAIN, - self::SHORT_URL_SHLINK_WITH_TITLE, + $shortUrlMeta, + $withDeviceLongUrls(self::SHORT_URL_CUSTOM_SLUG), + $withDeviceLongUrls(self::SHORT_URL_DOCS), + $withDeviceLongUrls(self::SHORT_URL_CUSTOM_DOMAIN), + $withDeviceLongUrls(self::SHORT_URL_CUSTOM_SLUG_AND_DOMAIN), + $withDeviceLongUrls(self::SHORT_URL_SHLINK_WITH_TITLE), ], 'valid_api_key']; yield [['startDate' => Chronos::parse('2018-12-01')->toAtomString()], [ - self::SHORT_URL_CUSTOM_DOMAIN, - self::SHORT_URL_CUSTOM_SLUG, - self::SHORT_URL_META, + $withDeviceLongUrls(self::SHORT_URL_CUSTOM_DOMAIN), + $withDeviceLongUrls(self::SHORT_URL_CUSTOM_SLUG), + $shortUrlMeta, ], 'valid_api_key']; yield [['endDate' => Chronos::parse('2018-12-01')->toAtomString()], [ - self::SHORT_URL_CUSTOM_SLUG_AND_DOMAIN, - self::SHORT_URL_SHLINK_WITH_TITLE, - self::SHORT_URL_DOCS, + $withDeviceLongUrls(self::SHORT_URL_CUSTOM_SLUG_AND_DOMAIN), + $withDeviceLongUrls(self::SHORT_URL_SHLINK_WITH_TITLE), + $withDeviceLongUrls(self::SHORT_URL_DOCS), ], 'valid_api_key']; yield [['tags' => ['foo']], [ - self::SHORT_URL_CUSTOM_DOMAIN, - self::SHORT_URL_META, - self::SHORT_URL_SHLINK_WITH_TITLE, + $withDeviceLongUrls(self::SHORT_URL_CUSTOM_DOMAIN), + $shortUrlMeta, + $withDeviceLongUrls(self::SHORT_URL_SHLINK_WITH_TITLE), ], 'valid_api_key']; yield [['tags' => ['bar']], [ - self::SHORT_URL_META, + $shortUrlMeta, ], 'valid_api_key']; yield [['tags' => ['foo', 'bar']], [ - self::SHORT_URL_CUSTOM_DOMAIN, - self::SHORT_URL_META, - self::SHORT_URL_SHLINK_WITH_TITLE, + $withDeviceLongUrls(self::SHORT_URL_CUSTOM_DOMAIN), + $shortUrlMeta, + $withDeviceLongUrls(self::SHORT_URL_SHLINK_WITH_TITLE), ], 'valid_api_key']; yield [['tags' => ['foo', 'bar'], 'tagsMode' => 'any'], [ - self::SHORT_URL_CUSTOM_DOMAIN, - self::SHORT_URL_META, - self::SHORT_URL_SHLINK_WITH_TITLE, + $withDeviceLongUrls(self::SHORT_URL_CUSTOM_DOMAIN), + $shortUrlMeta, + $withDeviceLongUrls(self::SHORT_URL_SHLINK_WITH_TITLE), ], 'valid_api_key']; yield [['tags' => ['foo', 'bar'], 'tagsMode' => 'all'], [ - self::SHORT_URL_META, + $shortUrlMeta, ], 'valid_api_key']; yield [['tags' => ['foo', 'bar', 'baz']], [ - self::SHORT_URL_CUSTOM_DOMAIN, - self::SHORT_URL_META, - self::SHORT_URL_SHLINK_WITH_TITLE, + $withDeviceLongUrls(self::SHORT_URL_CUSTOM_DOMAIN), + $shortUrlMeta, + $withDeviceLongUrls(self::SHORT_URL_SHLINK_WITH_TITLE), ], 'valid_api_key']; yield [['tags' => ['foo', 'bar', 'baz'], 'tagsMode' => 'all'], [], 'valid_api_key']; yield [['tags' => ['foo'], 'endDate' => Chronos::parse('2018-12-01')->toAtomString()], [ - self::SHORT_URL_SHLINK_WITH_TITLE, + $withDeviceLongUrls(self::SHORT_URL_SHLINK_WITH_TITLE), ], 'valid_api_key']; yield [['searchTerm' => 'alejandro'], [ - self::SHORT_URL_CUSTOM_DOMAIN, - self::SHORT_URL_META, + $withDeviceLongUrls(self::SHORT_URL_CUSTOM_DOMAIN), + $shortUrlMeta, ], 'valid_api_key']; yield [['searchTerm' => 'cool'], [ - self::SHORT_URL_SHLINK_WITH_TITLE, + $withDeviceLongUrls(self::SHORT_URL_SHLINK_WITH_TITLE), ], 'valid_api_key']; yield [['searchTerm' => 'example.com'], [ - self::SHORT_URL_CUSTOM_DOMAIN, + $withDeviceLongUrls(self::SHORT_URL_CUSTOM_DOMAIN), ], 'valid_api_key']; yield [[], [ - self::SHORT_URL_CUSTOM_SLUG, - self::SHORT_URL_META, - self::SHORT_URL_SHLINK_WITH_TITLE, + $withDeviceLongUrls(self::SHORT_URL_CUSTOM_SLUG), + $shortUrlMeta, + $withDeviceLongUrls(self::SHORT_URL_SHLINK_WITH_TITLE), ], 'author_api_key']; yield [[], [ - self::SHORT_URL_CUSTOM_DOMAIN, + $withDeviceLongUrls(self::SHORT_URL_CUSTOM_DOMAIN), ], 'domain_api_key']; } diff --git a/module/Rest/test-api/Fixtures/ShortUrlsFixture.php b/module/Rest/test-api/Fixtures/ShortUrlsFixture.php index 9a876463..2d45a7bb 100644 --- a/module/Rest/test-api/Fixtures/ShortUrlsFixture.php +++ b/module/Rest/test-api/Fixtures/ShortUrlsFixture.php @@ -9,6 +9,7 @@ use Doctrine\Common\DataFixtures\AbstractFixture; use Doctrine\Common\DataFixtures\DependentFixtureInterface; use Doctrine\Persistence\ObjectManager; use ReflectionObject; +use Shlinkio\Shlink\Core\Model\DeviceType; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlCreation; use Shlinkio\Shlink\Core\ShortUrl\Resolver\PersistenceShortUrlRelationResolver; @@ -48,6 +49,10 @@ class ShortUrlsFixture extends AbstractFixture implements DependentFixtureInterf 'apiKey' => $authorApiKey, 'longUrl' => 'https://blog.alejandrocelaya.com/2017/12/09/acmailer-7-0-the-most-important-release-in-a-long-time/', + 'deviceLongUrls' => [ + DeviceType::ANDROID->value => 'https://blog.alejandrocelaya.com/android', + DeviceType::IOS->value => 'https://blog.alejandrocelaya.com/ios', + ], 'tags' => ['foo', 'bar'], ]), $relationResolver), '2019-01-01 00:00:10'); $manager->persist($defShortUrl); diff --git a/module/Rest/test/Action/ShortUrl/CreateShortUrlActionTest.php b/module/Rest/test/Action/ShortUrl/CreateShortUrlActionTest.php index 246b2edf..15ce5389 100644 --- a/module/Rest/test/Action/ShortUrl/CreateShortUrlActionTest.php +++ b/module/Rest/test/Action/ShortUrl/CreateShortUrlActionTest.php @@ -37,7 +37,7 @@ class CreateShortUrlActionTest extends TestCase public function properShortcodeConversionReturnsData(): void { $apiKey = ApiKey::create(); - $shortUrl = ShortUrl::createEmpty(); + $shortUrl = ShortUrl::createFake(); $expectedMeta = $body = [ 'longUrl' => 'http://www.domain.com/foo/bar', 'validSince' => Chronos::now()->toAtomString(), diff --git a/module/Rest/test/Action/ShortUrl/EditShortUrlActionTest.php b/module/Rest/test/Action/ShortUrl/EditShortUrlActionTest.php index dde17ca6..ac788fa7 100644 --- a/module/Rest/test/Action/ShortUrl/EditShortUrlActionTest.php +++ b/module/Rest/test/Action/ShortUrl/EditShortUrlActionTest.php @@ -49,7 +49,7 @@ class EditShortUrlActionTest extends TestCase ->withParsedBody([ 'maxVisits' => 5, ]); - $this->shortUrlService->expects($this->once())->method('updateShortUrl')->willReturn(ShortUrl::createEmpty()); + $this->shortUrlService->expects($this->once())->method('updateShortUrl')->willReturn(ShortUrl::createFake()); $resp = $this->action->handle($request); diff --git a/module/Rest/test/Action/ShortUrl/SingleStepCreateShortUrlActionTest.php b/module/Rest/test/Action/ShortUrl/SingleStepCreateShortUrlActionTest.php index 14848696..42c185c9 100644 --- a/module/Rest/test/Action/ShortUrl/SingleStepCreateShortUrlActionTest.php +++ b/module/Rest/test/Action/ShortUrl/SingleStepCreateShortUrlActionTest.php @@ -43,7 +43,7 @@ class SingleStepCreateShortUrlActionTest extends TestCase ])->withAttribute(ApiKey::class, $apiKey); $this->urlShortener->expects($this->once())->method('shorten')->with( ShortUrlCreation::fromRawData(['apiKey' => $apiKey, 'longUrl' => 'http://foobar.com']), - )->willReturn(ShortUrl::createEmpty()); + )->willReturn(ShortUrl::createFake()); $resp = $this->action->handle($request);