diff --git a/CHANGELOG.md b/CHANGELOG.md index 5e288f92..626e7a03 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,25 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com), and this project adheres to [Semantic Versioning](https://semver.org). +## [3.5.3] - 2023-03-31 +### Added +* *Nothing* + +### Changed +* *Nothing* + +### Deprecated +* *Nothing* + +### Removed +* *Nothing* + +### Fixed +* [#1715](https://github.com/shlinkio/shlink/issues/1715) Fix short URL creation/edition allowing long URLs without schema. Now a validation error is thrown. +* [#1537](https://github.com/shlinkio/shlink/issues/1537) Fix incorrect list of tags being returned for some author-only API keys. +* [#1738](https://github.com/shlinkio/shlink/issues/1738) Fix memory leak when importing short URLs with many visits. + + ## [3.5.2] - 2023-02-16 ### Added * *Nothing* diff --git a/composer.json b/composer.json index 202ed25e..03ff1457 100644 --- a/composer.json +++ b/composer.json @@ -32,7 +32,6 @@ "laminas/laminas-inputfilter": "^2.24", "laminas/laminas-servicemanager": "^3.20", "laminas/laminas-stdlib": "^3.16", - "lcobucci/jwt": "^4.3", "league/uri": "^6.8", "lstrojny/functional-php": "^1.17", "mezzio/mezzio": "^3.15", @@ -46,7 +45,7 @@ "php-middleware/request-id": "^4.1", "pugx/shortid-php": "^1.1", "ramsey/uuid": "^4.7", - "shlinkio/shlink-common": "^5.3.1", + "shlinkio/shlink-common": "^5.4", "shlinkio/shlink-config": "^2.4", "shlinkio/shlink-event-dispatcher": "^2.6", "shlinkio/shlink-importer": "^5.0", diff --git a/config/autoload/entity-manager.local.php.dist b/config/autoload/entity-manager.local.php.dist index dadc49af..abe5dd87 100644 --- a/config/autoload/entity-manager.local.php.dist +++ b/config/autoload/entity-manager.local.php.dist @@ -15,6 +15,14 @@ return [ // 'dbname' => 'shlink_foo', 'charset' => 'utf8mb4', + // MariaDB +// 'user' => 'root', +// 'password' => 'root', +// 'driver' => 'pdo_mysql', +// 'host' => 'shlink_db_maria', +// 'dbname' => 'shlink_foo', +// 'charset' => 'utf8mb4', + // Postgres // 'user' => 'postgres', // 'password' => 'root', diff --git a/config/constants.php b/config/constants.php index cc802301..77f81fdd 100644 --- a/config/constants.php +++ b/config/constants.php @@ -13,6 +13,7 @@ const DEFAULT_REDIRECT_STATUS_CODE = RedirectStatus::STATUS_302; // Deprecated. const DEFAULT_REDIRECT_CACHE_LIFETIME = 30; const LOCAL_LOCK_FACTORY = 'Shlinkio\Shlink\LocalLockFactory'; const TITLE_TAG_VALUE = '/]*>(.*?)<\/title>/i'; // Matches the value inside a html title tag +const LOOSE_URI_MATCHER = '/(.+)\:\/\/(.+)/i'; // Matches anything starting with a schema. const DEFAULT_QR_CODE_SIZE = 300; const DEFAULT_QR_CODE_MARGIN = 0; const DEFAULT_QR_CODE_FORMAT = 'png'; diff --git a/data/migrations/Version20230303164233.php b/data/migrations/Version20230303164233.php new file mode 100644 index 00000000..3e64c03c --- /dev/null +++ b/data/migrations/Version20230303164233.php @@ -0,0 +1,28 @@ +getTable('visits'); + $this->skipIf($visits->hasIndex(self::INDEX_NAME)); + + $visits->dropIndex('IDX_visits_potential_bot'); // Old index + $visits->addIndex(['potential_bot'], self::INDEX_NAME); + } + + public function isTransactional(): bool + { + return ! ($this->connection->getDatabasePlatform() instanceof MySQLPlatform); + } +} diff --git a/docker/config/php.ini b/docker/config/php.ini index fca44924..248e508d 100644 --- a/docker/config/php.ini +++ b/docker/config/php.ini @@ -1,4 +1,4 @@ log_errors_max_len=0 zend.assertions=1 assert.exception=1 -memory_limit=256M +memory_limit=512M diff --git a/module/CLI/src/Command/Api/GenerateKeyCommand.php b/module/CLI/src/Command/Api/GenerateKeyCommand.php index 12adcd57..c89c4fbf 100644 --- a/module/CLI/src/Command/Api/GenerateKeyCommand.php +++ b/module/CLI/src/Command/Api/GenerateKeyCommand.php @@ -9,6 +9,7 @@ use Shlinkio\Shlink\CLI\ApiKey\RoleResolverInterface; use Shlinkio\Shlink\CLI\Util\ExitCodes; use Shlinkio\Shlink\CLI\Util\ShlinkTable; use Shlinkio\Shlink\Rest\ApiKey\Role; +use Shlinkio\Shlink\Rest\Entity\ApiKey; use Shlinkio\Shlink\Rest\Service\ApiKeyServiceInterface; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputInterface; @@ -99,7 +100,7 @@ class GenerateKeyCommand extends Command $io = new SymfonyStyle($input, $output); $io->success(sprintf('Generated API key: "%s"', $apiKey->toString())); - if (! $apiKey->isAdmin()) { + if (! ApiKey::isAdmin($apiKey)) { ShlinkTable::default($io)->render( ['Role name', 'Role metadata'], $apiKey->mapRoles(fn (Role $role, array $meta) => [$role->value, arrayToString($meta, 0)]), diff --git a/module/CLI/src/Command/Api/ListKeysCommand.php b/module/CLI/src/Command/Api/ListKeysCommand.php index 59f5b534..c7e31819 100644 --- a/module/CLI/src/Command/Api/ListKeysCommand.php +++ b/module/CLI/src/Command/Api/ListKeysCommand.php @@ -59,7 +59,7 @@ class ListKeysCommand extends Command $rowData[] = sprintf($messagePattern, $this->getEnabledSymbol($apiKey)); } $rowData[] = $expiration?->toAtomString() ?? '-'; - $rowData[] = $apiKey->isAdmin() ? 'Admin' : implode("\n", $apiKey->mapRoles( + $rowData[] = ApiKey::isAdmin($apiKey) ? 'Admin' : implode("\n", $apiKey->mapRoles( fn (Role $role, array $meta) => empty($meta) ? $role->toFriendlyName() diff --git a/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php b/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php index 6b1c790d..52d1eeb3 100644 --- a/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php +++ b/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php @@ -49,7 +49,7 @@ class ListShortUrlsCommandTest extends TestCase // The paginator will return more than one page $data = []; for ($i = 0; $i < 50; $i++) { - $data[] = ShortUrl::withLongUrl('url_' . $i); + $data[] = ShortUrl::withLongUrl('https://url_' . $i); } $this->shortUrlService->expects($this->exactly(3))->method('listShortUrls')->withAnyParameters() @@ -71,7 +71,7 @@ class ListShortUrlsCommandTest extends TestCase // The paginator will return more than one page $data = []; for ($i = 0; $i < 30; $i++) { - $data[] = ShortUrl::withLongUrl('url_' . $i); + $data[] = ShortUrl::withLongUrl('https://url_' . $i); } $this->shortUrlService->expects($this->once())->method('listShortUrls')->with( @@ -114,7 +114,7 @@ class ListShortUrlsCommandTest extends TestCase ShortUrlsParams::emptyInstance(), )->willReturn(new Paginator(new ArrayAdapter([ ShortUrl::create(ShortUrlCreation::fromRawData([ - 'longUrl' => 'foo.com', + 'longUrl' => 'https://foo.com', 'tags' => ['foo', 'bar', 'baz'], 'apiKey' => $apiKey, ])), diff --git a/module/Core/src/Importer/ImportedLinksProcessor.php b/module/Core/src/Importer/ImportedLinksProcessor.php index 7248e0d3..ef7633a7 100644 --- a/module/Core/src/Importer/ImportedLinksProcessor.php +++ b/module/Core/src/Importer/ImportedLinksProcessor.php @@ -18,7 +18,6 @@ use Shlinkio\Shlink\Importer\Model\ImportedShlinkOrphanVisit; use Shlinkio\Shlink\Importer\Model\ImportedShlinkUrl; use Shlinkio\Shlink\Importer\Model\ImportResult; use Shlinkio\Shlink\Importer\Params\ImportParams; -use Shlinkio\Shlink\Importer\Sources\ImportSource; use Symfony\Component\Console\Style\OutputStyle; use Symfony\Component\Console\Style\StyleInterface; use Throwable; @@ -55,8 +54,7 @@ class ImportedLinksProcessor implements ImportedLinksProcessorInterface private function importShortUrls(StyleInterface $io, iterable $shlinkUrls, ImportParams $params): void { $importShortCodes = $params->importShortCodes; - $source = $params->source; - $iterable = $this->batchHelper->wrapIterable($shlinkUrls, $source === ImportSource::SHLINK ? 10 : 100); + $iterable = $this->batchHelper->wrapIterable($shlinkUrls, $params->importVisits ? 10 : 100); foreach ($iterable as $importedUrl) { $skipOnShortCodeConflict = static fn (): bool => $io->choice(sprintf( @@ -82,7 +80,10 @@ class ImportedLinksProcessor implements ImportedLinksProcessorInterface continue; } - $resultMessage = $shortUrlImporting->importVisits($importedUrl->visits, $this->em); + $resultMessage = $shortUrlImporting->importVisits( + $this->batchHelper->wrapIterable($importedUrl->visits, 100), + $this->em, + ); $io->text(sprintf('%s: %s', $longUrl, $resultMessage)); } } diff --git a/module/Core/src/Importer/ShortUrlImporting.php b/module/Core/src/Importer/ShortUrlImporting.php index 0165c7c3..28c22a24 100644 --- a/module/Core/src/Importer/ShortUrlImporting.php +++ b/module/Core/src/Importer/ShortUrlImporting.php @@ -33,7 +33,7 @@ final class ShortUrlImporting */ public function importVisits(iterable $visits, EntityManagerInterface $em): string { - $mostRecentImportedDate = $this->shortUrl->mostRecentImportedVisitDate(); + $mostRecentImportedDate = $this->resolveShortUrl($em)->mostRecentImportedVisitDate(); $importedVisits = 0; foreach ($visits as $importedVisit) { @@ -42,7 +42,7 @@ final class ShortUrlImporting continue; } - $em->persist(Visit::fromImport($this->shortUrl, $importedVisit)); + $em->persist(Visit::fromImport($this->resolveShortUrl($em), $importedVisit)); $importedVisits++; } @@ -54,4 +54,14 @@ final class ShortUrlImporting ? sprintf('Imported with %s visits', $importedVisits) : sprintf('Skipped. Imported %s visits', $importedVisits); } + + private function resolveShortUrl(EntityManagerInterface $em): ShortUrl + { + // Instead of directly accessing wrapped ShortUrl entity, try to get it from the EM. + // With this, we will get the same entity from memory if it is known by the EM, but if it was cleared, the EM + // will fetch it again from the database, preventing errors at runtime. + // However, if the EM was not flushed yet, the entity will not be found by ID, but it is known by the EM. + // In that case, we fall back to wrapped ShortUrl entity directly. + return $em->find(ShortUrl::class, $this->shortUrl->getId()) ?? $this->shortUrl; + } } diff --git a/module/Core/src/ShortUrl/Entity/ShortUrl.php b/module/Core/src/ShortUrl/Entity/ShortUrl.php index 15f1998c..e5646bd4 100644 --- a/module/Core/src/ShortUrl/Entity/ShortUrl.php +++ b/module/Core/src/ShortUrl/Entity/ShortUrl.php @@ -68,7 +68,7 @@ class ShortUrl extends AbstractEntity */ public static function createFake(): self { - return self::withLongUrl('foo'); + return self::withLongUrl('https://foo'); } /** diff --git a/module/Core/src/ShortUrl/Model/Validation/ShortUrlInputFilter.php b/module/Core/src/ShortUrl/Model/Validation/ShortUrlInputFilter.php index 9c10d3ff..af7e8986 100644 --- a/module/Core/src/ShortUrl/Model/Validation/ShortUrlInputFilter.php +++ b/module/Core/src/ShortUrl/Model/Validation/ShortUrlInputFilter.php @@ -12,8 +12,11 @@ use Shlinkio\Shlink\Common\Validation; use Shlinkio\Shlink\Core\Options\UrlShortenerOptions; use Shlinkio\Shlink\Rest\Entity\ApiKey; +use function is_string; +use function preg_match; use function substr; +use const Shlinkio\Shlink\LOOSE_URI_MATCHER; use const Shlinkio\Shlink\MIN_SHORT_CODES_LENGTH; /** @@ -59,27 +62,13 @@ class ShortUrlInputFilter extends InputFilter private function initialize(bool $requireLongUrl, UrlShortenerOptions $options): void { - $longUrlNotEmptyCommonOptions = [ - Validator\NotEmpty::OBJECT, - Validator\NotEmpty::SPACE, - 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, - ])); + $longUrlInput->getValidatorChain()->merge($this->longUrlValidators()); $this->add($longUrlInput); $deviceLongUrlsInput = $this->createInput(self::DEVICE_LONG_URLS, false); $deviceLongUrlsInput->getValidatorChain()->attach( - new DeviceLongUrlsValidator(new Validator\NotEmpty([ - ...$longUrlNotEmptyCommonOptions, - ...($requireLongUrl ? [Validator\NotEmpty::NULL] : []), - ])), + new DeviceLongUrlsValidator($this->longUrlValidators(allowNull: ! $requireLongUrl)), ); $this->add($deviceLongUrlsInput); @@ -129,4 +118,25 @@ class ShortUrlInputFilter extends InputFilter $this->add($this->createBooleanInput(self::CRAWLABLE, false)); } + + private function longUrlValidators(bool $allowNull = false): Validator\ValidatorChain + { + $emptyModifiers = [ + Validator\NotEmpty::OBJECT, + Validator\NotEmpty::SPACE, + Validator\NotEmpty::EMPTY_ARRAY, + Validator\NotEmpty::BOOLEAN, + Validator\NotEmpty::STRING, + ]; + if (! $allowNull) { + $emptyModifiers[] = Validator\NotEmpty::NULL; + } + + return (new Validator\ValidatorChain()) + ->attach(new Validator\NotEmpty($emptyModifiers)) + ->attach(new Validator\Callback( + // Non-strings is always allowed. Other validators will take care of those + static fn (mixed $value) => ! is_string($value) || preg_match(LOOSE_URI_MATCHER, $value) === 1, + )); + } } diff --git a/module/Core/src/ShortUrl/Repository/CrawlableShortCodesQuery.php b/module/Core/src/ShortUrl/Repository/CrawlableShortCodesQuery.php index 7b3821d8..89aef69e 100644 --- a/module/Core/src/ShortUrl/Repository/CrawlableShortCodesQuery.php +++ b/module/Core/src/ShortUrl/Repository/CrawlableShortCodesQuery.php @@ -20,7 +20,8 @@ class CrawlableShortCodesQuery extends EntitySpecificationRepository implements ->from(ShortUrl::class, 's') ->where($qb->expr()->eq('s.crawlable', ':crawlable')) ->setParameter('crawlable', true) - ->setMaxResults($blockSize); + ->setMaxResults($blockSize) + ->orderBy('s.shortCode'); $page = 0; do { diff --git a/module/Core/src/Tag/Model/OrderableField.php b/module/Core/src/Tag/Model/OrderableField.php index 818099de..b7a9509f 100644 --- a/module/Core/src/Tag/Model/OrderableField.php +++ b/module/Core/src/Tag/Model/OrderableField.php @@ -4,8 +4,6 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Tag\Model; -use function Shlinkio\Shlink\Core\camelCaseToSnakeCase; - enum OrderableField: string { case TAG = 'tag'; @@ -15,20 +13,12 @@ enum OrderableField: string /** @deprecated Use VISITS instead */ case VISITS_COUNT = 'visitsCount'; - public static function isAggregateField(string $field): bool + public static function toSnakeCaseValidField(?string $field): self { - $parsed = self::tryFrom($field); - return $parsed !== null && $parsed !== self::TAG; - } - - public static function toSnakeCaseValidField(?string $field): string - { - $parsed = $field !== null ? self::tryFrom($field) : self::VISITS; - $normalized = match ($parsed) { + $parsed = $field !== null ? self::tryFrom($field) : self::TAG; + return match ($parsed) { self::VISITS_COUNT, null => self::VISITS, default => $parsed, }; - - return camelCaseToSnakeCase($normalized->value); } } diff --git a/module/Core/src/Tag/Repository/TagRepository.php b/module/Core/src/Tag/Repository/TagRepository.php index 4e9eafac..68e5df4b 100644 --- a/module/Core/src/Tag/Repository/TagRepository.php +++ b/module/Core/src/Tag/Repository/TagRepository.php @@ -15,10 +15,11 @@ use Shlinkio\Shlink\Core\Tag\Model\TagsListFiltering; use Shlinkio\Shlink\Core\Tag\Spec\CountTagsWithName; use Shlinkio\Shlink\Rest\ApiKey\Role; use Shlinkio\Shlink\Rest\ApiKey\Spec\WithApiKeySpecsEnsuringJoin; -use Shlinkio\Shlink\Rest\ApiKey\Spec\WithInlinedApiKeySpecsEnsuringJoin; use Shlinkio\Shlink\Rest\Entity\ApiKey; +use function Functional\each; use function Functional\map; +use function Shlinkio\Shlink\Core\camelCaseToSnakeCase; use const PHP_INT_MAX; @@ -42,106 +43,90 @@ class TagRepository extends EntitySpecificationRepository implements TagReposito */ public function findTagsWithInfo(?TagsListFiltering $filtering = null): array { - $orderField = $filtering?->orderBy?->field; - $orderDir = $filtering?->orderBy?->direction; - $orderMainQuery = $orderField !== null && OrderableField::isAggregateField($orderField); - - $subQb = $this->createQueryBuilder('t'); - $subQb->select('t.id', 't.name'); - - if (! $orderMainQuery) { - $subQb->orderBy('t.name', $orderDir ?? 'ASC') - ->setMaxResults($filtering?->limit ?? PHP_INT_MAX) - ->setFirstResult($filtering?->offset ?? 0); - // TODO Check if applying limit/offset ot visits sub-queries is needed with large amounts of tags - } - - $conn = $this->getEntityManager()->getConnection(); - $buildVisitsSubQuery = static function (bool $excludeBots, string $aggregateAlias) use ($conn) { - $visitsSubQuery = $conn->createQueryBuilder(); - $commonJoinCondition = $visitsSubQuery->expr()->eq('v.short_url_id', 's.id'); - $visitsJoin = ! $excludeBots - ? $commonJoinCondition - : $visitsSubQuery->expr()->and( - $commonJoinCondition, - $visitsSubQuery->expr()->eq('v.potential_bot', $conn->quote('0')), - ); - - return $visitsSubQuery - ->select('st.tag_id AS tag_id', 'COUNT(DISTINCT v.id) AS ' . $aggregateAlias) - ->from('visits', 'v') - ->join('v', 'short_urls', 's', $visitsJoin) // @phpstan-ignore-line - ->join('s', 'short_urls_in_tags', 'st', $visitsSubQuery->expr()->eq('st.short_url_id', 's.id')) - ->groupBy('st.tag_id'); - }; - $allVisitsSubQuery = $buildVisitsSubQuery(false, 'visits'); - $nonBotVisitsSubQuery = $buildVisitsSubQuery(true, 'non_bot_visits'); - - $searchTerm = $filtering?->searchTerm; - if ($searchTerm !== null) { - $subQb->andWhere($subQb->expr()->like('t.name', $conn->quote('%' . $searchTerm . '%'))); - // TODO Check if applying this to all sub-queries makes it faster or slower - } - + $orderField = OrderableField::toSnakeCaseValidField($filtering?->orderBy?->field); + $orderDir = $filtering?->orderBy?->direction ?? 'ASC'; $apiKey = $filtering?->apiKey; - $applyApiKeyToNativeQuery = static fn (?ApiKey $apiKey, NativeQueryBuilder $nativeQueryBuilder) => + $conn = $this->getEntityManager()->getConnection(); + + $applyApiKeyToNativeQb = static fn (NativeQueryBuilder $qb) => $apiKey?->mapRoles(static fn (Role $role, array $meta) => match ($role) { - Role::DOMAIN_SPECIFIC => $nativeQueryBuilder->andWhere( - $nativeQueryBuilder->expr()->eq('s.domain_id', $conn->quote(Role::domainIdFromMeta($meta))), + Role::DOMAIN_SPECIFIC => $qb->andWhere( + $qb->expr()->eq('s.domain_id', $conn->quote(Role::domainIdFromMeta($meta))), ), - Role::AUTHORED_SHORT_URLS => $nativeQueryBuilder->andWhere( - $nativeQueryBuilder->expr()->eq('s.author_api_key_id', $conn->quote($apiKey->getId())), + Role::AUTHORED_SHORT_URLS => $qb->andWhere( + $qb->expr()->eq('s.author_api_key_id', $conn->quote($apiKey->getId())), ), }); + // For admins and when no API key is present, we'll return tags which are not linked to any short URL + $joiningMethod = ApiKey::isAdmin($apiKey) ? 'leftJoin' : 'join'; + $tagsSubQb = $conn->createQueryBuilder(); + $tagsSubQb + ->select('t.id AS tag_id', 't.name AS tag', 'COUNT(DISTINCT s.id) AS short_urls_count') + ->from('tags', 't') + ->groupBy('t.id', 't.name') + ->{$joiningMethod}('t', 'short_urls_in_tags', 'st', $tagsSubQb->expr()->eq('st.tag_id', 't.id')) + ->{$joiningMethod}('st', 'short_urls', 's', $tagsSubQb->expr()->eq('st.short_url_id', 's.id')); + + $searchTerm = $filtering?->searchTerm; + if ($searchTerm !== null) { + $tagsSubQb->andWhere($tagsSubQb->expr()->like('t.name', $conn->quote('%' . $searchTerm . '%'))); + } + + $buildVisitsSubQb = static function (bool $excludeBots, string $aggregateAlias) use ($conn) { + $visitsSubQb = $conn->createQueryBuilder(); + $commonJoinCondition = $visitsSubQb->expr()->eq('v.short_url_id', 's.id'); + $visitsJoin = ! $excludeBots + ? $commonJoinCondition + : $visitsSubQb->expr()->and( + $commonJoinCondition, + $visitsSubQb->expr()->eq('v.potential_bot', $conn->quote('0')), + ); + + return $visitsSubQb + ->select('st.tag_id AS tag_id', 'COUNT(DISTINCT v.id) AS ' . $aggregateAlias) + ->from('visits', 'v') + ->join('v', 'short_urls', 's', $visitsJoin) // @phpstan-ignore-line + ->join('s', 'short_urls_in_tags', 'st', $visitsSubQb->expr()->eq('st.short_url_id', 's.id')) + ->groupBy('st.tag_id'); + }; + $allVisitsSubQb = $buildVisitsSubQb(false, 'visits'); + $nonBotVisitsSubQb = $buildVisitsSubQb(true, 'non_bot_visits'); + // Apply API key specification to all sub-queries - $this->applySpecification($subQb, new WithInlinedApiKeySpecsEnsuringJoin($apiKey), 't'); - $applyApiKeyToNativeQuery($apiKey, $allVisitsSubQuery); - $applyApiKeyToNativeQuery($apiKey, $nonBotVisitsSubQuery); + each([$tagsSubQb, $allVisitsSubQb, $nonBotVisitsSubQb], $applyApiKeyToNativeQb); // A native query builder needs to be used here, because DQL and ORM query builders do not support // sub-queries at "from" and "join" level. // If no sub-query is used, the whole list is loaded even with pagination, making it very inefficient. - $nativeQb = $conn->createQueryBuilder(); - $nativeQb + $mainQb = $conn->createQueryBuilder(); + $mainQb ->select( - 't.id_0 AS id', - 't.name_1 AS name', + 't.tag AS tag', 'COALESCE(v.visits, 0) AS visits', // COALESCE required for postgres to properly order - 'COALESCE(v2.non_bot_visits, 0) AS non_bot_visits', // COALESCE required for postgres to properly order - 'COUNT(DISTINCT s.id) AS short_urls_count', + 'COALESCE(b.non_bot_visits, 0) AS non_bot_visits', + 'COALESCE(t.short_urls_count, 0) AS short_urls_count', ) - ->from('(' . $subQb->getQuery()->getSQL() . ')', 't') // @phpstan-ignore-line - ->leftJoin('t', 'short_urls_in_tags', 'st', $nativeQb->expr()->eq('t.id_0', 'st.tag_id')) - ->leftJoin('st', 'short_urls', 's', $nativeQb->expr()->eq('s.id', 'st.short_url_id')) - ->leftJoin('t', '(' . $allVisitsSubQuery->getSQL() . ')', 'v', $nativeQb->expr()->eq('t.id_0', 'v.tag_id')) - ->leftJoin('t', '(' . $nonBotVisitsSubQuery->getSQL() . ')', 'v2', $nativeQb->expr()->eq( - 't.id_0', - 'v2.tag_id', - )) - ->groupBy('t.id_0', 't.name_1', 'v.visits', 'v2.non_bot_visits'); + ->from('(' . $tagsSubQb->getSQL() . ')', 't') + ->leftJoin('t', '(' . $allVisitsSubQb->getSQL() . ')', 'v', $mainQb->expr()->eq('t.tag_id', 'v.tag_id')) + ->leftJoin('t', '(' . $nonBotVisitsSubQb->getSQL() . ')', 'b', $mainQb->expr()->eq('t.tag_id', 'b.tag_id')) + ->setMaxResults($filtering?->limit ?? PHP_INT_MAX) + ->setFirstResult($filtering?->offset ?? 0); - // Apply API key role conditions to the native query too, as they will affect the amounts on the aggregates - $applyApiKeyToNativeQuery($apiKey, $nativeQb); - - if ($orderMainQuery) { - $nativeQb - ->orderBy(OrderableField::toSnakeCaseValidField($orderField), $orderDir ?? 'ASC') - ->setMaxResults($filtering?->limit ?? PHP_INT_MAX) - ->setFirstResult($filtering?->offset ?? 0); + $mainQb->orderBy(camelCaseToSnakeCase($orderField->value), $orderDir); + if ($orderField !== OrderableField::TAG) { + // Add ordering by tag name, as a fallback in case of same amounts + $mainQb->addOrderBy('tag', 'ASC'); } - // Add ordering by tag name, as a fallback in case of same amount, or as default ordering - $nativeQb->addOrderBy('t.name_1', $orderMainQuery || $orderDir === null ? 'ASC' : $orderDir); - $rsm = new ResultSetMappingBuilder($this->getEntityManager()); - $rsm->addScalarResult('name', 'tag'); + $rsm->addScalarResult('tag', 'tag'); $rsm->addScalarResult('visits', 'visits'); $rsm->addScalarResult('non_bot_visits', 'nonBotVisits'); $rsm->addScalarResult('short_urls_count', 'shortUrlsCount'); return map( - $this->getEntityManager()->createNativeQuery($nativeQb->getSQL(), $rsm)->getResult(), + $this->getEntityManager()->createNativeQuery($mainQb->getSQL(), $rsm)->getResult(), TagInfo::fromRawData(...), ); } diff --git a/module/Core/src/Tag/TagService.php b/module/Core/src/Tag/TagService.php index d50ced75..ea9a4e8b 100644 --- a/module/Core/src/Tag/TagService.php +++ b/module/Core/src/Tag/TagService.php @@ -59,7 +59,7 @@ class TagService implements TagServiceInterface */ public function deleteTags(array $tagNames, ?ApiKey $apiKey = null): void { - if ($apiKey !== null && ! $apiKey->isAdmin()) { + if (! ApiKey::isAdmin($apiKey)) { throw ForbiddenTagOperationException::forDeletion(); } @@ -75,7 +75,7 @@ class TagService implements TagServiceInterface */ public function renameTag(TagRenaming $renaming, ?ApiKey $apiKey = null): Tag { - if ($apiKey !== null && ! $apiKey->isAdmin()) { + if (! ApiKey::isAdmin($apiKey)) { throw ForbiddenTagOperationException::forRenaming(); } diff --git a/module/Core/src/Util/DoctrineBatchHelper.php b/module/Core/src/Util/DoctrineBatchHelper.php index 5591ddb2..93d2d022 100644 --- a/module/Core/src/Util/DoctrineBatchHelper.php +++ b/module/Core/src/Util/DoctrineBatchHelper.php @@ -17,12 +17,14 @@ class DoctrineBatchHelper implements DoctrineBatchHelperInterface } /** + * @template T + * @param iterable $resultSet + * @return iterable * @throws Throwable */ public function wrapIterable(iterable $resultSet, int $batchSize): iterable { $iteration = 0; - $this->em->beginTransaction(); try { @@ -33,7 +35,6 @@ class DoctrineBatchHelper implements DoctrineBatchHelperInterface } } catch (Throwable $e) { $this->em->rollback(); - throw $e; } diff --git a/module/Core/test-api/Action/RobotsTest.php b/module/Core/test-api/Action/RobotsTest.php index 490c82fb..9b550fe3 100644 --- a/module/Core/test-api/Action/RobotsTest.php +++ b/module/Core/test-api/Action/RobotsTest.php @@ -22,8 +22,8 @@ class RobotsTest extends ApiTestCase # https://www.robotstxt.org/orig.html User-agent: * - Allow: /custom Allow: /abc123 + Allow: /custom Disallow: / ROBOTS, $body, diff --git a/module/Core/test-db/Domain/Repository/DomainRepositoryTest.php b/module/Core/test-db/Domain/Repository/DomainRepositoryTest.php index 24824ee4..58817f38 100644 --- a/module/Core/test-db/Domain/Repository/DomainRepositoryTest.php +++ b/module/Core/test-db/Domain/Repository/DomainRepositoryTest.php @@ -132,7 +132,7 @@ class DomainRepositoryTest extends DatabaseTestCase { return ShortUrl::create( ShortUrlCreation::fromRawData( - ['domain' => $domain->authority, 'apiKey' => $apiKey, 'longUrl' => 'foo'], + ['domain' => $domain->authority, 'apiKey' => $apiKey, 'longUrl' => 'https://foo'], ), new class ($domain) implements ShortUrlRelationResolverInterface { public function __construct(private Domain $domain) diff --git a/module/Core/test-db/ShortUrl/Repository/CrawlableShortCodesQueryTest.php b/module/Core/test-db/ShortUrl/Repository/CrawlableShortCodesQueryTest.php index 47f13567..d630520b 100644 --- a/module/Core/test-db/ShortUrl/Repository/CrawlableShortCodesQueryTest.php +++ b/module/Core/test-db/ShortUrl/Repository/CrawlableShortCodesQueryTest.php @@ -24,7 +24,7 @@ class CrawlableShortCodesQueryTest extends DatabaseTestCase public function invokingQueryReturnsExpectedResult(): void { $createShortUrl = fn (bool $crawlable) => ShortUrl::create( - ShortUrlCreation::fromRawData(['crawlable' => $crawlable, 'longUrl' => 'foo.com']), + ShortUrlCreation::fromRawData(['crawlable' => $crawlable, 'longUrl' => 'https://foo.com']), ); $shortUrl1 = $createShortUrl(true); diff --git a/module/Core/test-db/ShortUrl/Repository/ShortUrlListRepositoryTest.php b/module/Core/test-db/ShortUrl/Repository/ShortUrlListRepositoryTest.php index 97c6dd22..46c08d25 100644 --- a/module/Core/test-db/ShortUrl/Repository/ShortUrlListRepositoryTest.php +++ b/module/Core/test-db/ShortUrl/Repository/ShortUrlListRepositoryTest.php @@ -43,7 +43,7 @@ class ShortUrlListRepositoryTest extends DatabaseTestCase { $count = 5; for ($i = 0; $i < $count; $i++) { - $this->getEntityManager()->persist(ShortUrl::withLongUrl((string) $i)); + $this->getEntityManager()->persist(ShortUrl::withLongUrl('https://' . $i)); } $this->getEntityManager()->flush(); @@ -54,12 +54,12 @@ class ShortUrlListRepositoryTest extends DatabaseTestCase public function findListProperlyFiltersResult(): void { $foo = ShortUrl::create( - ShortUrlCreation::fromRawData(['longUrl' => 'foo', 'tags' => ['bar']]), + ShortUrlCreation::fromRawData(['longUrl' => 'https://foo', 'tags' => ['bar']]), $this->relationResolver, ); $this->getEntityManager()->persist($foo); - $bar = ShortUrl::withLongUrl('bar'); + $bar = ShortUrl::withLongUrl('https://bar'); $visits = map(range(0, 5), function () use ($bar) { $visit = Visit::forValidShortUrl($bar, Visitor::botInstance()); $this->getEntityManager()->persist($visit); @@ -69,7 +69,7 @@ class ShortUrlListRepositoryTest extends DatabaseTestCase $bar->setVisits(new ArrayCollection($visits)); $this->getEntityManager()->persist($bar); - $foo2 = ShortUrl::withLongUrl('foo_2'); + $foo2 = ShortUrl::withLongUrl('https://foo_2'); $visits2 = map(range(0, 3), function () use ($foo2) { $visit = Visit::forValidShortUrl($foo2, Visitor::emptyInstance()); $this->getEntityManager()->persist($visit); @@ -147,7 +147,7 @@ class ShortUrlListRepositoryTest extends DatabaseTestCase #[Test] public function findListProperlyMapsFieldNamesToColumnNamesWhenOrdering(): void { - $urls = ['a', 'z', 'c', 'b']; + $urls = ['https://a', 'https://z', 'https://c', 'https://b']; foreach ($urls as $url) { $this->getEntityManager()->persist(ShortUrl::withLongUrl($url)); } @@ -159,37 +159,37 @@ class ShortUrlListRepositoryTest extends DatabaseTestCase ); self::assertCount(count($urls), $result); - self::assertEquals('a', $result[0]->getLongUrl()); - self::assertEquals('b', $result[1]->getLongUrl()); - self::assertEquals('c', $result[2]->getLongUrl()); - self::assertEquals('z', $result[3]->getLongUrl()); + self::assertEquals('https://a', $result[0]->getLongUrl()); + self::assertEquals('https://b', $result[1]->getLongUrl()); + self::assertEquals('https://c', $result[2]->getLongUrl()); + self::assertEquals('https://z', $result[3]->getLongUrl()); } #[Test] public function findListReturnsOnlyThoseWithMatchingTags(): void { $shortUrl1 = ShortUrl::create(ShortUrlCreation::fromRawData([ - 'longUrl' => 'foo1', + 'longUrl' => 'https://foo1', 'tags' => ['foo', 'bar'], ]), $this->relationResolver); $this->getEntityManager()->persist($shortUrl1); $shortUrl2 = ShortUrl::create(ShortUrlCreation::fromRawData([ - 'longUrl' => 'foo2', + 'longUrl' => 'https://foo2', 'tags' => ['foo', 'baz'], ]), $this->relationResolver); $this->getEntityManager()->persist($shortUrl2); $shortUrl3 = ShortUrl::create(ShortUrlCreation::fromRawData([ - 'longUrl' => 'foo3', + 'longUrl' => 'https://foo3', 'tags' => ['foo'], ]), $this->relationResolver); $this->getEntityManager()->persist($shortUrl3); $shortUrl4 = ShortUrl::create(ShortUrlCreation::fromRawData([ - 'longUrl' => 'foo4', + 'longUrl' => 'https://foo4', 'tags' => ['bar', 'baz'], ]), $this->relationResolver); $this->getEntityManager()->persist($shortUrl4); $shortUrl5 = ShortUrl::create(ShortUrlCreation::fromRawData([ - 'longUrl' => 'foo5', + 'longUrl' => 'https://foo5', 'tags' => ['bar', 'baz'], ]), $this->relationResolver); $this->getEntityManager()->persist($shortUrl5); @@ -278,17 +278,17 @@ class ShortUrlListRepositoryTest extends DatabaseTestCase public function findListReturnsOnlyThoseWithMatchingDomains(): void { $shortUrl1 = ShortUrl::create(ShortUrlCreation::fromRawData([ - 'longUrl' => 'foo1', + 'longUrl' => 'https://foo1', 'domain' => null, ]), $this->relationResolver); $this->getEntityManager()->persist($shortUrl1); $shortUrl2 = ShortUrl::create(ShortUrlCreation::fromRawData([ - 'longUrl' => 'foo2', + 'longUrl' => 'https://foo2', 'domain' => null, ]), $this->relationResolver); $this->getEntityManager()->persist($shortUrl2); $shortUrl3 = ShortUrl::create(ShortUrlCreation::fromRawData([ - 'longUrl' => 'foo3', + 'longUrl' => 'https://foo3', 'domain' => 'another.com', ]), $this->relationResolver); $this->getEntityManager()->persist($shortUrl3); @@ -314,22 +314,22 @@ class ShortUrlListRepositoryTest extends DatabaseTestCase public function findListReturnsOnlyThoseWithoutExcludedUrls(): void { $shortUrl1 = ShortUrl::create(ShortUrlCreation::fromRawData([ - 'longUrl' => 'foo1', + 'longUrl' => 'https://foo1', 'validUntil' => Chronos::now()->addDays(1)->toAtomString(), 'maxVisits' => 100, ]), $this->relationResolver); $this->getEntityManager()->persist($shortUrl1); $shortUrl2 = ShortUrl::create(ShortUrlCreation::fromRawData([ - 'longUrl' => 'foo2', + 'longUrl' => 'https://foo2', 'validUntil' => Chronos::now()->subDays(1)->toAtomString(), ]), $this->relationResolver); $this->getEntityManager()->persist($shortUrl2); $shortUrl3 = ShortUrl::create(ShortUrlCreation::fromRawData([ - 'longUrl' => 'foo3', + 'longUrl' => 'https://foo3', ]), $this->relationResolver); $this->getEntityManager()->persist($shortUrl3); $shortUrl4 = ShortUrl::create(ShortUrlCreation::fromRawData([ - 'longUrl' => 'foo4', + 'longUrl' => 'https://foo4', 'maxVisits' => 3, ]), $this->relationResolver); $this->getEntityManager()->persist($shortUrl4); diff --git a/module/Core/test-db/ShortUrl/Repository/ShortUrlRepositoryTest.php b/module/Core/test-db/ShortUrl/Repository/ShortUrlRepositoryTest.php index f2cce0f7..074acdd4 100644 --- a/module/Core/test-db/ShortUrl/Repository/ShortUrlRepositoryTest.php +++ b/module/Core/test-db/ShortUrl/Repository/ShortUrlRepositoryTest.php @@ -34,16 +34,18 @@ class ShortUrlRepositoryTest extends DatabaseTestCase #[Test] public function findOneWithDomainFallbackReturnsProperData(): void { - $regularOne = ShortUrl::create(ShortUrlCreation::fromRawData(['customSlug' => 'Foo', 'longUrl' => 'foo'])); + $regularOne = ShortUrl::create( + ShortUrlCreation::fromRawData(['customSlug' => 'Foo', 'longUrl' => 'https://foo']), + ); $this->getEntityManager()->persist($regularOne); $withDomain = ShortUrl::create(ShortUrlCreation::fromRawData( - ['domain' => 'example.com', 'customSlug' => 'domain-short-code', 'longUrl' => 'foo'], + ['domain' => 'example.com', 'customSlug' => 'domain-short-code', 'longUrl' => 'https://foo'], )); $this->getEntityManager()->persist($withDomain); $withDomainDuplicatingRegular = ShortUrl::create(ShortUrlCreation::fromRawData( - ['domain' => 's.test', 'customSlug' => 'Foo', 'longUrl' => 'foo_with_domain'], + ['domain' => 's.test', 'customSlug' => 'Foo', 'longUrl' => 'https://foo_with_domain'], )); $this->getEntityManager()->persist($withDomainDuplicatingRegular); @@ -102,13 +104,13 @@ class ShortUrlRepositoryTest extends DatabaseTestCase public function shortCodeIsInUseLooksForShortUrlInProperSetOfTables(): void { $shortUrlWithoutDomain = ShortUrl::create( - ShortUrlCreation::fromRawData(['customSlug' => 'my-cool-slug', 'longUrl' => 'foo']), + ShortUrlCreation::fromRawData(['customSlug' => 'my-cool-slug', 'longUrl' => 'https://foo']), ); $this->getEntityManager()->persist($shortUrlWithoutDomain); - $shortUrlWithDomain = ShortUrl::create( - ShortUrlCreation::fromRawData(['domain' => 's.test', 'customSlug' => 'another-slug', 'longUrl' => 'foo']), - ); + $shortUrlWithDomain = ShortUrl::create(ShortUrlCreation::fromRawData( + ['domain' => 's.test', 'customSlug' => 'another-slug', 'longUrl' => 'https://foo'], + )); $this->getEntityManager()->persist($shortUrlWithDomain); $this->getEntityManager()->flush(); @@ -131,13 +133,13 @@ class ShortUrlRepositoryTest extends DatabaseTestCase public function findOneLooksForShortUrlInProperSetOfTables(): void { $shortUrlWithoutDomain = ShortUrl::create( - ShortUrlCreation::fromRawData(['customSlug' => 'my-cool-slug', 'longUrl' => 'foo']), + ShortUrlCreation::fromRawData(['customSlug' => 'my-cool-slug', 'longUrl' => 'https://foo']), ); $this->getEntityManager()->persist($shortUrlWithoutDomain); - $shortUrlWithDomain = ShortUrl::create( - ShortUrlCreation::fromRawData(['domain' => 's.test', 'customSlug' => 'another-slug', 'longUrl' => 'foo']), - ); + $shortUrlWithDomain = ShortUrl::create(ShortUrlCreation::fromRawData( + ['domain' => 's.test', 'customSlug' => 'another-slug', 'longUrl' => 'https://foo'], + )); $this->getEntityManager()->persist($shortUrlWithDomain); $this->getEntityManager()->flush(); @@ -157,14 +159,14 @@ class ShortUrlRepositoryTest extends DatabaseTestCase #[Test] public function findOneMatchingReturnsNullForNonExistingShortUrls(): void { - self::assertNull($this->repo->findOneMatching(ShortUrlCreation::fromRawData(['longUrl' => 'foobar']))); + self::assertNull($this->repo->findOneMatching(ShortUrlCreation::fromRawData(['longUrl' => 'https://foobar']))); self::assertNull($this->repo->findOneMatching( - ShortUrlCreation::fromRawData(['longUrl' => 'foobar', 'tags' => ['foo', 'bar']]), + ShortUrlCreation::fromRawData(['longUrl' => 'https://foobar', 'tags' => ['foo', 'bar']]), )); self::assertNull($this->repo->findOneMatching(ShortUrlCreation::fromRawData([ 'validSince' => Chronos::parse('2020-03-05 20:18:30'), 'customSlug' => 'this_slug_does_not_exist', - 'longUrl' => 'foobar', + 'longUrl' => 'https://foobar', 'tags' => ['foo', 'bar'], ]))); } @@ -175,49 +177,54 @@ class ShortUrlRepositoryTest extends DatabaseTestCase $start = Chronos::parse('2020-03-05 20:18:30'); $end = Chronos::parse('2021-03-05 20:18:30'); - $shortUrl = ShortUrl::create( - ShortUrlCreation::fromRawData(['validSince' => $start, 'longUrl' => 'foo', 'tags' => ['foo', 'bar']]), - $this->relationResolver, - ); + $shortUrl = ShortUrl::create(ShortUrlCreation::fromRawData( + ['validSince' => $start, 'longUrl' => 'https://foo', 'tags' => ['foo', 'bar']], + ), $this->relationResolver); $this->getEntityManager()->persist($shortUrl); - $shortUrl2 = ShortUrl::create(ShortUrlCreation::fromRawData(['validUntil' => $end, 'longUrl' => 'bar'])); + $shortUrl2 = ShortUrl::create( + ShortUrlCreation::fromRawData(['validUntil' => $end, 'longUrl' => 'https://bar']), + ); $this->getEntityManager()->persist($shortUrl2); $shortUrl3 = ShortUrl::create( - ShortUrlCreation::fromRawData(['validSince' => $start, 'validUntil' => $end, 'longUrl' => 'baz']), + ShortUrlCreation::fromRawData(['validSince' => $start, 'validUntil' => $end, 'longUrl' => 'https://baz']), ); $this->getEntityManager()->persist($shortUrl3); $shortUrl4 = ShortUrl::create( - ShortUrlCreation::fromRawData(['customSlug' => 'custom', 'validUntil' => $end, 'longUrl' => 'foo']), + ShortUrlCreation::fromRawData(['customSlug' => 'custom', 'validUntil' => $end, 'longUrl' => 'https://foo']), ); $this->getEntityManager()->persist($shortUrl4); - $shortUrl5 = ShortUrl::create(ShortUrlCreation::fromRawData(['maxVisits' => 3, 'longUrl' => 'foo'])); + $shortUrl5 = ShortUrl::create(ShortUrlCreation::fromRawData(['maxVisits' => 3, 'longUrl' => 'https://foo'])); $this->getEntityManager()->persist($shortUrl5); - $shortUrl6 = ShortUrl::create(ShortUrlCreation::fromRawData(['domain' => 's.test', 'longUrl' => 'foo'])); + $shortUrl6 = ShortUrl::create( + ShortUrlCreation::fromRawData(['domain' => 's.test', 'longUrl' => 'https://foo']), + ); $this->getEntityManager()->persist($shortUrl6); $this->getEntityManager()->flush(); self::assertSame( $shortUrl, - $this->repo->findOneMatching( - ShortUrlCreation::fromRawData(['validSince' => $start, 'longUrl' => 'foo', 'tags' => ['foo', 'bar']]), - ), + $this->repo->findOneMatching(ShortUrlCreation::fromRawData( + ['validSince' => $start, 'longUrl' => 'https://foo', 'tags' => ['foo', 'bar']], + )), ); self::assertSame( $shortUrl2, - $this->repo->findOneMatching(ShortUrlCreation::fromRawData(['validUntil' => $end, 'longUrl' => 'bar'])), + $this->repo->findOneMatching( + ShortUrlCreation::fromRawData(['validUntil' => $end, 'longUrl' => 'https://bar']), + ), ); self::assertSame( $shortUrl3, $this->repo->findOneMatching(ShortUrlCreation::fromRawData([ 'validSince' => $start, 'validUntil' => $end, - 'longUrl' => 'baz', + 'longUrl' => 'https://baz', ])), ); self::assertSame( @@ -225,16 +232,18 @@ class ShortUrlRepositoryTest extends DatabaseTestCase $this->repo->findOneMatching(ShortUrlCreation::fromRawData([ 'customSlug' => 'custom', 'validUntil' => $end, - 'longUrl' => 'foo', + 'longUrl' => 'https://foo', ])), ); self::assertSame( $shortUrl5, - $this->repo->findOneMatching(ShortUrlCreation::fromRawData(['maxVisits' => 3, 'longUrl' => 'foo'])), + $this->repo->findOneMatching(ShortUrlCreation::fromRawData(['maxVisits' => 3, 'longUrl' => 'https://foo'])), ); self::assertSame( $shortUrl6, - $this->repo->findOneMatching(ShortUrlCreation::fromRawData(['domain' => 's.test', 'longUrl' => 'foo'])), + $this->repo->findOneMatching( + ShortUrlCreation::fromRawData(['domain' => 's.test', 'longUrl' => 'https://foo']), + ), ); } @@ -244,7 +253,7 @@ class ShortUrlRepositoryTest extends DatabaseTestCase $start = Chronos::parse('2020-03-05 20:18:30'); $tags = ['foo', 'bar']; $meta = ShortUrlCreation::fromRawData( - ['validSince' => $start, 'maxVisits' => 50, 'longUrl' => 'foo', 'tags' => $tags], + ['validSince' => $start, 'maxVisits' => 50, 'longUrl' => 'https://foo', 'tags' => $tags], ); $shortUrl1 = ShortUrl::create($meta, $this->relationResolver); @@ -293,14 +302,14 @@ class ShortUrlRepositoryTest extends DatabaseTestCase 'validSince' => $start, 'apiKey' => $apiKey, 'domain' => $rightDomain->authority, - 'longUrl' => 'foo', + 'longUrl' => 'https://foo', 'tags' => ['foo', 'bar'], ]), $this->relationResolver); $this->getEntityManager()->persist($shortUrl); $nonDomainShortUrl = ShortUrl::create(ShortUrlCreation::fromRawData([ 'apiKey' => $apiKey, - 'longUrl' => 'non-domain', + 'longUrl' => 'https://non-domain', ]), $this->relationResolver); $this->getEntityManager()->persist($nonDomainShortUrl); @@ -308,26 +317,26 @@ class ShortUrlRepositoryTest extends DatabaseTestCase self::assertSame( $shortUrl, - $this->repo->findOneMatching( - ShortUrlCreation::fromRawData(['validSince' => $start, 'longUrl' => 'foo', 'tags' => ['foo', 'bar']]), - ), + $this->repo->findOneMatching(ShortUrlCreation::fromRawData( + ['validSince' => $start, 'longUrl' => 'https://foo', 'tags' => ['foo', 'bar']], + )), ); self::assertSame($shortUrl, $this->repo->findOneMatching(ShortUrlCreation::fromRawData([ 'validSince' => $start, 'apiKey' => $apiKey, - 'longUrl' => 'foo', + 'longUrl' => 'https://foo', 'tags' => ['foo', 'bar'], ]))); self::assertSame($shortUrl, $this->repo->findOneMatching(ShortUrlCreation::fromRawData([ 'validSince' => $start, 'apiKey' => $adminApiKey, - 'longUrl' => 'foo', + 'longUrl' => 'https://foo', 'tags' => ['foo', 'bar'], ]))); self::assertNull($this->repo->findOneMatching(ShortUrlCreation::fromRawData([ 'validSince' => $start, 'apiKey' => $otherApiKey, - 'longUrl' => 'foo', + 'longUrl' => 'https://foo', 'tags' => ['foo', 'bar'], ]))); @@ -336,7 +345,7 @@ class ShortUrlRepositoryTest extends DatabaseTestCase $this->repo->findOneMatching(ShortUrlCreation::fromRawData([ 'validSince' => $start, 'domain' => $rightDomain->authority, - 'longUrl' => 'foo', + 'longUrl' => 'https://foo', 'tags' => ['foo', 'bar'], ])), ); @@ -346,7 +355,7 @@ class ShortUrlRepositoryTest extends DatabaseTestCase 'validSince' => $start, 'domain' => $rightDomain->authority, 'apiKey' => $rightDomainApiKey, - 'longUrl' => 'foo', + 'longUrl' => 'https://foo', 'tags' => ['foo', 'bar'], ])), ); @@ -356,7 +365,7 @@ class ShortUrlRepositoryTest extends DatabaseTestCase 'validSince' => $start, 'domain' => $rightDomain->authority, 'apiKey' => $apiKey, - 'longUrl' => 'foo', + 'longUrl' => 'https://foo', 'tags' => ['foo', 'bar'], ])), ); @@ -365,7 +374,7 @@ class ShortUrlRepositoryTest extends DatabaseTestCase 'validSince' => $start, 'domain' => $rightDomain->authority, 'apiKey' => $wrongDomainApiKey, - 'longUrl' => 'foo', + 'longUrl' => 'https://foo', 'tags' => ['foo', 'bar'], ])), ); @@ -374,20 +383,20 @@ class ShortUrlRepositoryTest extends DatabaseTestCase $nonDomainShortUrl, $this->repo->findOneMatching(ShortUrlCreation::fromRawData([ 'apiKey' => $apiKey, - 'longUrl' => 'non-domain', + 'longUrl' => 'https://non-domain', ])), ); self::assertSame( $nonDomainShortUrl, $this->repo->findOneMatching(ShortUrlCreation::fromRawData([ 'apiKey' => $adminApiKey, - 'longUrl' => 'non-domain', + 'longUrl' => 'https://non-domain', ])), ); self::assertNull( $this->repo->findOneMatching(ShortUrlCreation::fromRawData([ 'apiKey' => $otherApiKey, - 'longUrl' => 'non-domain', + 'longUrl' => 'https://non-domain', ])), ); } @@ -395,8 +404,8 @@ class ShortUrlRepositoryTest extends DatabaseTestCase #[Test] public function importedShortUrlsAreFoundWhenExpected(): void { - $buildImported = static fn (string $shortCode, ?String $domain = null) => - new ImportedShlinkUrl(ImportSource::BITLY, 'foo', [], Chronos::now(), $domain, $shortCode, null); + $buildImported = static fn (string $shortCode, ?string $domain = null) => + new ImportedShlinkUrl(ImportSource::BITLY, 'https://foo', [], Chronos::now(), $domain, $shortCode, null); $shortUrlWithoutDomain = ShortUrl::fromImport($buildImported('my-cool-slug'), true); $this->getEntityManager()->persist($shortUrlWithoutDomain); diff --git a/module/Core/test-db/Tag/Repository/TagRepositoryTest.php b/module/Core/test-db/Tag/Repository/TagRepositoryTest.php index 47b4b41d..6cccf199 100644 --- a/module/Core/test-db/Tag/Repository/TagRepositoryTest.php +++ b/module/Core/test-db/Tag/Repository/TagRepositoryTest.php @@ -74,7 +74,7 @@ class TagRepositoryTest extends DatabaseTestCase [$firstUrlTags] = array_chunk($names, 3); $secondUrlTags = [$names[0]]; $metaWithTags = static fn (array $tags, ?ApiKey $apiKey) => ShortUrlCreation::fromRawData( - ['longUrl' => 'longUrl', 'tags' => $tags, 'apiKey' => $apiKey], + ['longUrl' => 'https://longUrl', 'tags' => $tags, 'apiKey' => $apiKey], ); $shortUrl = ShortUrl::create($metaWithTags($firstUrlTags, $apiKey), $this->relationResolver); @@ -240,15 +240,14 @@ class TagRepositoryTest extends DatabaseTestCase [$firstUrlTags, $secondUrlTags] = array_chunk($names, 3); - $shortUrl = ShortUrl::create( - ShortUrlCreation::fromRawData(['apiKey' => $authorApiKey, 'longUrl' => 'longUrl', 'tags' => $firstUrlTags]), - $this->relationResolver, - ); + $shortUrl = ShortUrl::create(ShortUrlCreation::fromRawData( + ['apiKey' => $authorApiKey, 'longUrl' => 'https://longUrl', 'tags' => $firstUrlTags], + ), $this->relationResolver); $this->getEntityManager()->persist($shortUrl); $shortUrl2 = ShortUrl::create( ShortUrlCreation::fromRawData( - ['domain' => $domain->authority, 'longUrl' => 'longUrl', 'tags' => $secondUrlTags], + ['domain' => $domain->authority, 'longUrl' => 'https://longUrl', 'tags' => $secondUrlTags], ), $this->relationResolver, ); diff --git a/module/Core/test-db/Visit/Repository/VisitRepositoryTest.php b/module/Core/test-db/Visit/Repository/VisitRepositoryTest.php index 3e0ca9bf..6eb2fe4e 100644 --- a/module/Core/test-db/Visit/Repository/VisitRepositoryTest.php +++ b/module/Core/test-db/Visit/Repository/VisitRepositoryTest.php @@ -266,7 +266,7 @@ class VisitRepositoryTest extends DatabaseTestCase $this->getEntityManager()->persist($apiKey1); $shortUrl = ShortUrl::create( ShortUrlCreation::fromRawData( - ['apiKey' => $apiKey1, 'domain' => $domain->authority, 'longUrl' => 'longUrl'], + ['apiKey' => $apiKey1, 'domain' => $domain->authority, 'longUrl' => 'https://longUrl'], ), $this->relationResolver, ); @@ -275,13 +275,15 @@ class VisitRepositoryTest extends DatabaseTestCase $apiKey2 = ApiKey::fromMeta(ApiKeyMeta::withRoles(RoleDefinition::forAuthoredShortUrls())); $this->getEntityManager()->persist($apiKey2); - $shortUrl2 = ShortUrl::create(ShortUrlCreation::fromRawData(['apiKey' => $apiKey2, 'longUrl' => 'longUrl'])); + $shortUrl2 = ShortUrl::create( + ShortUrlCreation::fromRawData(['apiKey' => $apiKey2, 'longUrl' => 'https://longUrl']), + ); $this->getEntityManager()->persist($shortUrl2); $this->createVisitsForShortUrl($shortUrl2, 5); $shortUrl3 = ShortUrl::create( ShortUrlCreation::fromRawData( - ['apiKey' => $apiKey2, 'domain' => $domain->authority, 'longUrl' => 'longUrl'], + ['apiKey' => $apiKey2, 'domain' => $domain->authority, 'longUrl' => 'https://longUrl'], ), $this->relationResolver, ); @@ -320,7 +322,7 @@ class VisitRepositoryTest extends DatabaseTestCase #[Test] public function findOrphanVisitsReturnsExpectedResult(): void { - $shortUrl = ShortUrl::create(ShortUrlCreation::fromRawData(['longUrl' => 'longUrl'])); + $shortUrl = ShortUrl::create(ShortUrlCreation::fromRawData(['longUrl' => 'https://longUrl'])); $this->getEntityManager()->persist($shortUrl); $this->createVisitsForShortUrl($shortUrl, 7); @@ -369,7 +371,7 @@ class VisitRepositoryTest extends DatabaseTestCase #[Test] public function countOrphanVisitsReturnsExpectedResult(): void { - $shortUrl = ShortUrl::create(ShortUrlCreation::fromRawData(['longUrl' => 'longUrl'])); + $shortUrl = ShortUrl::create(ShortUrlCreation::fromRawData(['longUrl' => 'https://longUrl'])); $this->getEntityManager()->persist($shortUrl); $this->createVisitsForShortUrl($shortUrl, 7); @@ -406,15 +408,15 @@ class VisitRepositoryTest extends DatabaseTestCase #[Test] public function findNonOrphanVisitsReturnsExpectedResult(): void { - $shortUrl = ShortUrl::create(ShortUrlCreation::fromRawData(['longUrl' => '1'])); + $shortUrl = ShortUrl::create(ShortUrlCreation::fromRawData(['longUrl' => 'https://1'])); $this->getEntityManager()->persist($shortUrl); $this->createVisitsForShortUrl($shortUrl, 7); - $shortUrl2 = ShortUrl::create(ShortUrlCreation::fromRawData(['longUrl' => '2'])); + $shortUrl2 = ShortUrl::create(ShortUrlCreation::fromRawData(['longUrl' => 'https://2'])); $this->getEntityManager()->persist($shortUrl2); $this->createVisitsForShortUrl($shortUrl2, 4); - $shortUrl3 = ShortUrl::create(ShortUrlCreation::fromRawData(['longUrl' => '3'])); + $shortUrl3 = ShortUrl::create(ShortUrlCreation::fromRawData(['longUrl' => 'https://3'])); $this->getEntityManager()->persist($shortUrl3); $this->createVisitsForShortUrl($shortUrl3, 10); @@ -473,7 +475,7 @@ class VisitRepositoryTest extends DatabaseTestCase ?ApiKey $apiKey = null, ): array { $shortUrl = ShortUrl::create(ShortUrlCreation::fromRawData([ - ShortUrlInputFilter::LONG_URL => 'longUrl', + ShortUrlInputFilter::LONG_URL => 'https://longUrl', ShortUrlInputFilter::TAGS => $tags, ShortUrlInputFilter::API_KEY => $apiKey, ]), $this->relationResolver); @@ -487,7 +489,7 @@ class VisitRepositoryTest extends DatabaseTestCase $shortUrlWithDomain = ShortUrl::create(ShortUrlCreation::fromRawData([ 'customSlug' => $shortCode, 'domain' => $domain, - 'longUrl' => 'longUrl', + 'longUrl' => 'https://longUrl', ])); $this->getEntityManager()->persist($shortUrlWithDomain); $this->createVisitsForShortUrl($shortUrlWithDomain, 3); diff --git a/module/Core/test/EventDispatcher/Mercure/NotifyNewShortUrlToMercureTest.php b/module/Core/test/EventDispatcher/Mercure/NotifyNewShortUrlToMercureTest.php index ceec7235..20d6830d 100644 --- a/module/Core/test/EventDispatcher/Mercure/NotifyNewShortUrlToMercureTest.php +++ b/module/Core/test/EventDispatcher/Mercure/NotifyNewShortUrlToMercureTest.php @@ -58,7 +58,7 @@ class NotifyNewShortUrlToMercureTest extends TestCase #[Test] public function expectedNotificationIsPublished(): void { - $shortUrl = ShortUrl::withLongUrl('longUrl'); + $shortUrl = ShortUrl::withLongUrl('https://longUrl'); $update = Update::forTopicAndPayload('', []); $this->em->expects($this->once())->method('find')->with(ShortUrl::class, '123')->willReturn($shortUrl); @@ -75,7 +75,7 @@ class NotifyNewShortUrlToMercureTest extends TestCase #[Test] public function messageIsPrintedIfPublishingFails(): void { - $shortUrl = ShortUrl::withLongUrl('longUrl'); + $shortUrl = ShortUrl::withLongUrl('https://longUrl'); $update = Update::forTopicAndPayload('', []); $e = new Exception('Error'); diff --git a/module/Core/test/EventDispatcher/PublishingUpdatesGeneratorTest.php b/module/Core/test/EventDispatcher/PublishingUpdatesGeneratorTest.php index d2d5b0e2..9d28f2cd 100644 --- a/module/Core/test/EventDispatcher/PublishingUpdatesGeneratorTest.php +++ b/module/Core/test/EventDispatcher/PublishingUpdatesGeneratorTest.php @@ -37,7 +37,7 @@ class PublishingUpdatesGeneratorTest extends TestCase { $shortUrl = ShortUrl::create(ShortUrlCreation::fromRawData([ 'customSlug' => 'foo', - 'longUrl' => 'longUrl', + 'longUrl' => 'https://longUrl', 'title' => $title, ])); $visit = Visit::forValidShortUrl($shortUrl, Visitor::emptyInstance()); @@ -50,7 +50,7 @@ class PublishingUpdatesGeneratorTest extends TestCase 'shortUrl' => [ 'shortCode' => $shortUrl->getShortCode(), 'shortUrl' => 'http:/' . $shortUrl->getShortCode(), - 'longUrl' => 'longUrl', + 'longUrl' => 'https://longUrl', 'deviceLongUrls' => $shortUrl->deviceLongUrls(), 'dateCreated' => $shortUrl->getDateCreated()->toAtomString(), 'visitsCount' => 0, @@ -115,7 +115,7 @@ class PublishingUpdatesGeneratorTest extends TestCase { $shortUrl = ShortUrl::create(ShortUrlCreation::fromRawData([ 'customSlug' => 'foo', - 'longUrl' => 'longUrl', + 'longUrl' => 'https://longUrl', 'title' => 'The title', ])); @@ -125,7 +125,7 @@ class PublishingUpdatesGeneratorTest extends TestCase self::assertEquals(['shortUrl' => [ 'shortCode' => $shortUrl->getShortCode(), 'shortUrl' => 'http:/' . $shortUrl->getShortCode(), - 'longUrl' => 'longUrl', + 'longUrl' => 'https://longUrl', 'deviceLongUrls' => $shortUrl->deviceLongUrls(), 'dateCreated' => $shortUrl->getDateCreated()->toAtomString(), 'visitsCount' => 0, diff --git a/module/Core/test/EventDispatcher/RabbitMq/NotifyNewShortUrlToRabbitMqTest.php b/module/Core/test/EventDispatcher/RabbitMq/NotifyNewShortUrlToRabbitMqTest.php index a3bd9fcc..fc44fd87 100644 --- a/module/Core/test/EventDispatcher/RabbitMq/NotifyNewShortUrlToRabbitMqTest.php +++ b/module/Core/test/EventDispatcher/RabbitMq/NotifyNewShortUrlToRabbitMqTest.php @@ -70,7 +70,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('longUrl'), + ShortUrl::withLongUrl('https://longUrl'), ); $this->updatesGenerator->expects($this->once())->method('newShortUrlUpdate')->with( $this->isInstanceOf(ShortUrl::class), @@ -87,7 +87,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('longUrl'), + ShortUrl::withLongUrl('https://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 e8a0f0d5..0002d3b1 100644 --- a/module/Core/test/EventDispatcher/RabbitMq/NotifyVisitToRabbitMqTest.php +++ b/module/Core/test/EventDispatcher/RabbitMq/NotifyVisitToRabbitMqTest.php @@ -98,7 +98,7 @@ class NotifyVisitToRabbitMqTest extends TestCase yield 'non-orphan visit' => [ Visit::forValidShortUrl( ShortUrl::create(ShortUrlCreation::fromRawData([ - 'longUrl' => 'foo', + 'longUrl' => 'https://foo', 'customSlug' => 'bar', ])), $visitor, @@ -152,7 +152,7 @@ class NotifyVisitToRabbitMqTest extends TestCase { yield 'legacy non-orphan visit' => [ true, - $visit = Visit::forValidShortUrl(ShortUrl::withLongUrl('longUrl'), Visitor::emptyInstance()), + $visit = Visit::forValidShortUrl(ShortUrl::withLongUrl('https://longUrl'), Visitor::emptyInstance()), noop(...), function (MockObject & PublishingHelperInterface $helper) use ($visit): void { $helper->method('publishUpdate')->with(self::callback(function (Update $update) use ($visit): bool { @@ -183,7 +183,7 @@ class NotifyVisitToRabbitMqTest extends TestCase ]; yield 'non-legacy non-orphan visit' => [ false, - Visit::forValidShortUrl(ShortUrl::withLongUrl('longUrl'), Visitor::emptyInstance()), + Visit::forValidShortUrl(ShortUrl::withLongUrl('https://longUrl'), Visitor::emptyInstance()), function (MockObject & PublishingUpdatesGeneratorInterface $updatesGenerator): void { $update = Update::forTopicAndPayload('', []); $updatesGenerator->expects(self::never())->method('newOrphanVisitUpdate'); diff --git a/module/Core/test/EventDispatcher/RedisPubSub/NotifyNewShortUrlToRedisTest.php b/module/Core/test/EventDispatcher/RedisPubSub/NotifyNewShortUrlToRedisTest.php index 894abceb..abbe23b9 100644 --- a/module/Core/test/EventDispatcher/RedisPubSub/NotifyNewShortUrlToRedisTest.php +++ b/module/Core/test/EventDispatcher/RedisPubSub/NotifyNewShortUrlToRedisTest.php @@ -54,7 +54,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('longUrl'), + ShortUrl::withLongUrl('https://longUrl'), ); $this->updatesGenerator->expects($this->once())->method('newShortUrlUpdate')->with( $this->isInstanceOf(ShortUrl::class), diff --git a/module/Core/test/Importer/ImportedLinksProcessorTest.php b/module/Core/test/Importer/ImportedLinksProcessorTest.php index c8132699..ff8eebc6 100644 --- a/module/Core/test/Importer/ImportedLinksProcessorTest.php +++ b/module/Core/test/Importer/ImportedLinksProcessorTest.php @@ -7,6 +7,7 @@ namespace ShlinkioTest\Shlink\Core\Importer; use Cake\Chronos\Chronos; use Doctrine\Common\Collections\ArrayCollection; use Doctrine\ORM\EntityManagerInterface; +use PHPUnit\Framework\Assert; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\MockObject\MockObject; @@ -67,9 +68,9 @@ class ImportedLinksProcessorTest extends TestCase public function newUrlsWithNoErrorsAreAllPersisted(): void { $urls = [ - new ImportedShlinkUrl(ImportSource::BITLY, 'foo', [], Chronos::now(), null, 'foo', null), - new ImportedShlinkUrl(ImportSource::BITLY, 'bar', [], Chronos::now(), null, 'bar', 'foo'), - new ImportedShlinkUrl(ImportSource::BITLY, 'baz', [], Chronos::now(), null, 'baz', null), + new ImportedShlinkUrl(ImportSource::BITLY, 'https://foo', [], Chronos::now(), null, 'foo', null), + new ImportedShlinkUrl(ImportSource::BITLY, 'https://bar', [], Chronos::now(), null, 'bar', 'foo'), + new ImportedShlinkUrl(ImportSource::BITLY, 'https://baz', [], Chronos::now(), null, 'baz', null), ]; $expectedCalls = count($urls); @@ -90,9 +91,9 @@ class ImportedLinksProcessorTest extends TestCase public function newUrlsWithErrorsAreSkipped(): void { $urls = [ - new ImportedShlinkUrl(ImportSource::BITLY, 'foo', [], Chronos::now(), null, 'foo', null), - new ImportedShlinkUrl(ImportSource::BITLY, 'bar', [], Chronos::now(), null, 'bar', 'foo'), - new ImportedShlinkUrl(ImportSource::BITLY, 'baz', [], Chronos::now(), null, 'baz', null), + new ImportedShlinkUrl(ImportSource::BITLY, 'https://foo', [], Chronos::now(), null, 'foo', null), + new ImportedShlinkUrl(ImportSource::BITLY, 'https://bar', [], Chronos::now(), null, 'bar', 'foo'), + new ImportedShlinkUrl(ImportSource::BITLY, 'https://baz', [], Chronos::now(), null, 'baz', null), ]; $this->em->method('getRepository')->with(ShortUrl::class)->willReturn($this->repo); @@ -117,17 +118,19 @@ class ImportedLinksProcessorTest extends TestCase public function alreadyImportedUrlsAreSkipped(): void { $urls = [ - new ImportedShlinkUrl(ImportSource::BITLY, 'foo', [], Chronos::now(), null, 'foo', null), - new ImportedShlinkUrl(ImportSource::BITLY, 'bar', [], Chronos::now(), null, 'bar', null), - new ImportedShlinkUrl(ImportSource::BITLY, 'baz', [], Chronos::now(), null, 'baz', null), - new ImportedShlinkUrl(ImportSource::BITLY, 'baz2', [], Chronos::now(), null, 'baz2', null), - new ImportedShlinkUrl(ImportSource::BITLY, 'baz3', [], Chronos::now(), null, 'baz3', null), + new ImportedShlinkUrl(ImportSource::BITLY, 'https://foo', [], Chronos::now(), null, 'foo', null), + new ImportedShlinkUrl(ImportSource::BITLY, 'https://bar', [], Chronos::now(), null, 'bar', null), + new ImportedShlinkUrl(ImportSource::BITLY, 'https://baz', [], Chronos::now(), null, 'baz', null), + new ImportedShlinkUrl(ImportSource::BITLY, 'https://baz2', [], Chronos::now(), null, 'baz2', null), + new ImportedShlinkUrl(ImportSource::BITLY, 'https://baz3', [], Chronos::now(), null, 'baz3', null), ]; $this->em->method('getRepository')->with(ShortUrl::class)->willReturn($this->repo); $this->repo->expects($this->exactly(count($urls)))->method('findOneByImportedUrl')->willReturnCallback( - fn (ImportedShlinkUrl $url): ?ShortUrl - => contains(['foo', 'baz2', 'baz3'], $url->longUrl) ? ShortUrl::fromImport($url, true) : null, + fn (ImportedShlinkUrl $url): ?ShortUrl => contains( + ['https://foo', 'https://baz2', 'https://baz3'], + $url->longUrl, + ) ? ShortUrl::fromImport($url, true) : null, ); $this->shortCodeHelper->expects($this->exactly(2))->method('ensureShortCodeUniqueness')->willReturn(true); $this->em->expects($this->exactly(2))->method('persist')->with($this->isInstanceOf(ShortUrl::class)); @@ -143,11 +146,11 @@ class ImportedLinksProcessorTest extends TestCase public function nonUniqueShortCodesAreAskedToUser(): void { $urls = [ - new ImportedShlinkUrl(ImportSource::BITLY, 'foo', [], Chronos::now(), null, 'foo', null), - new ImportedShlinkUrl(ImportSource::BITLY, 'bar', [], Chronos::now(), null, 'bar', null), - new ImportedShlinkUrl(ImportSource::BITLY, 'baz', [], Chronos::now(), null, 'baz', 'foo'), - new ImportedShlinkUrl(ImportSource::BITLY, 'baz2', [], Chronos::now(), null, 'baz2', null), - new ImportedShlinkUrl(ImportSource::BITLY, 'baz3', [], Chronos::now(), null, 'baz3', 'bar'), + new ImportedShlinkUrl(ImportSource::BITLY, 'https://foo', [], Chronos::now(), null, 'foo', null), + new ImportedShlinkUrl(ImportSource::BITLY, 'https://bar', [], Chronos::now(), null, 'bar', null), + new ImportedShlinkUrl(ImportSource::BITLY, 'https://baz', [], Chronos::now(), null, 'baz', 'foo'), + new ImportedShlinkUrl(ImportSource::BITLY, 'https://baz2', [], Chronos::now(), null, 'baz2', null), + new ImportedShlinkUrl(ImportSource::BITLY, 'https://baz3', [], Chronos::now(), null, 'baz3', 'bar'), ]; $this->em->method('getRepository')->with(ShortUrl::class)->willReturn($this->repo); @@ -182,6 +185,7 @@ class ImportedLinksProcessorTest extends TestCase $this->em->expects($this->exactly($amountOfPersistedVisits + ($foundShortUrl === null ? 1 : 0)))->method( 'persist', )->with($this->callback(fn (object $arg) => $arg instanceof ShortUrl || $arg instanceof Visit)); + $this->em->expects($this->any())->method('find')->willReturn(null); $this->io->expects($this->once())->method('text')->with($this->stringContains($expectedOutput)); $this->processor->process($this->io, ImportResult::withShortUrls([$importedUrl]), $this->buildParams()); @@ -191,7 +195,7 @@ class ImportedLinksProcessorTest extends TestCase { $now = Chronos::now(); $createImportedUrl = static fn (array $visits) => - new ImportedShlinkUrl(ImportSource::BITLY, 's', [], $now, null, 's', null, $visits); + new ImportedShlinkUrl(ImportSource::BITLY, 'https://s', [], $now, null, 's', null, $visits); yield 'new short URL' => [$createImportedUrl([ new ImportedShlinkVisit('', '', $now, null), @@ -227,6 +231,32 @@ class ImportedLinksProcessorTest extends TestCase ]; } + #[Test, DataProvider('provideFoundShortUrls')] + public function visitsArePersistedWithProperShortUrl(?ShortUrl $foundShortUrl): void + { + $originalShortUrl = ShortUrl::withLongUrl('https://foo'); + + $this->em->method('getRepository')->with(ShortUrl::class)->willReturn($this->repo); + $this->repo->expects($this->once())->method('findOneByImportedUrl')->willReturn($originalShortUrl); + $this->em->expects($this->exactly(2))->method('find')->willReturn($foundShortUrl); + $this->em->expects($this->once())->method('persist')->willReturnCallback( + static fn (Visit $visit) => Assert::assertSame($foundShortUrl ?? $originalShortUrl, $visit->getShortUrl()), + ); + + $now = Chronos::now(); + $this->processor->process($this->io, ImportResult::withShortUrls([ + new ImportedShlinkUrl(ImportSource::SHLINK, 'https://s', [], $now, null, 's', null, [ + new ImportedShlinkVisit('', '', $now, null), + ]), + ]), $this->buildParams()); + } + + public static function provideFoundShortUrls(): iterable + { + yield [null]; + yield [ShortUrl::withLongUrl('https://bar')]; + } + /** * @param iterable $visits */ diff --git a/module/Core/test/ShortUrl/Entity/ShortUrlTest.php b/module/Core/test/ShortUrl/Entity/ShortUrlTest.php index 54e21461..bd83fd9a 100644 --- a/module/Core/test/ShortUrl/Entity/ShortUrlTest.php +++ b/module/Core/test/ShortUrl/Entity/ShortUrlTest.php @@ -43,7 +43,9 @@ class ShortUrlTest extends TestCase public static function provideInvalidShortUrls(): iterable { yield 'with custom slug' => [ - ShortUrl::create(ShortUrlCreation::fromRawData(['customSlug' => 'custom-slug', 'longUrl' => 'longUrl'])), + ShortUrl::create( + ShortUrlCreation::fromRawData(['customSlug' => 'custom-slug', 'longUrl' => 'https://longUrl']), + ), 'The short code cannot be regenerated on ShortUrls where a custom slug was provided.', ]; yield 'already persisted' => [ @@ -68,7 +70,7 @@ class ShortUrlTest extends TestCase { yield 'no custom slug' => [ShortUrl::createFake()]; yield 'imported with custom slug' => [ShortUrl::fromImport( - new ImportedShlinkUrl(ImportSource::BITLY, 'longUrl', [], Chronos::now(), null, 'custom-slug', null), + new ImportedShlinkUrl(ImportSource::BITLY, 'https://url', [], Chronos::now(), null, 'custom-slug', null), true, )]; } @@ -77,7 +79,7 @@ class ShortUrlTest extends TestCase public function shortCodesHaveExpectedLength(?int $length, int $expectedLength): void { $shortUrl = ShortUrl::create(ShortUrlCreation::fromRawData( - [ShortUrlInputFilter::SHORT_CODE_LENGTH => $length, 'longUrl' => 'longUrl'], + [ShortUrlInputFilter::SHORT_CODE_LENGTH => $length, 'longUrl' => 'https://longUrl'], )); self::assertEquals($expectedLength, strlen($shortUrl->getShortCode())); @@ -92,30 +94,30 @@ class ShortUrlTest extends TestCase #[Test] public function deviceLongUrlsAreUpdated(): void { - $shortUrl = ShortUrl::withLongUrl('foo'); + $shortUrl = ShortUrl::withLongUrl('https://foo'); $shortUrl->update(ShortUrlEdition::fromRawData([ ShortUrlInputFilter::DEVICE_LONG_URLS => [ - DeviceType::ANDROID->value => 'android', - DeviceType::IOS->value => 'ios', + DeviceType::ANDROID->value => 'https://android', + DeviceType::IOS->value => 'https://ios', ], ])); self::assertEquals([ - DeviceType::ANDROID->value => 'android', - DeviceType::IOS->value => 'ios', + DeviceType::ANDROID->value => 'https://android', + DeviceType::IOS->value => 'https://ios', DeviceType::DESKTOP->value => null, ], $shortUrl->deviceLongUrls()); $shortUrl->update(ShortUrlEdition::fromRawData([ ShortUrlInputFilter::DEVICE_LONG_URLS => [ DeviceType::ANDROID->value => null, - DeviceType::DESKTOP->value => 'desktop', + DeviceType::DESKTOP->value => 'https://desktop', ], ])); self::assertEquals([ DeviceType::ANDROID->value => null, - DeviceType::IOS->value => 'ios', - DeviceType::DESKTOP->value => 'desktop', + DeviceType::IOS->value => 'https://ios', + DeviceType::DESKTOP->value => 'https://desktop', ], $shortUrl->deviceLongUrls()); $shortUrl->update(ShortUrlEdition::fromRawData([ @@ -127,7 +129,7 @@ class ShortUrlTest extends TestCase self::assertEquals([ DeviceType::ANDROID->value => null, DeviceType::IOS->value => null, - DeviceType::DESKTOP->value => 'desktop', + DeviceType::DESKTOP->value => 'https://desktop', ], $shortUrl->deviceLongUrls()); } @@ -137,7 +139,7 @@ class ShortUrlTest extends TestCase $range = range(1, 1000); // Use a "big" number to reduce false negatives $allFor = static fn (ShortUrlMode $mode): bool => every($range, static function () use ($mode): bool { $shortUrl = ShortUrl::create(ShortUrlCreation::fromRawData( - [ShortUrlInputFilter::LONG_URL => 'foo'], + [ShortUrlInputFilter::LONG_URL => 'https://foo'], new UrlShortenerOptions(mode: $mode), )); $shortCode = $shortUrl->getShortCode(); diff --git a/module/Core/test/ShortUrl/Helper/ShortUrlStringifierTest.php b/module/Core/test/ShortUrl/Helper/ShortUrlStringifierTest.php index 1ccd6eac..e74d6182 100644 --- a/module/Core/test/ShortUrl/Helper/ShortUrlStringifierTest.php +++ b/module/Core/test/ShortUrl/Helper/ShortUrlStringifierTest.php @@ -29,7 +29,7 @@ class ShortUrlStringifierTest extends TestCase { $shortUrlWithShortCode = fn (string $shortCode, ?string $domain = null) => ShortUrl::create( ShortUrlCreation::fromRawData([ - 'longUrl' => 'longUrl', + 'longUrl' => 'https://longUrl', 'customSlug' => $shortCode, 'domain' => $domain, ]), diff --git a/module/Core/test/ShortUrl/Middleware/ExtraPathRedirectMiddlewareTest.php b/module/Core/test/ShortUrl/Middleware/ExtraPathRedirectMiddlewareTest.php index bbe8e770..3965fe18 100644 --- a/module/Core/test/ShortUrl/Middleware/ExtraPathRedirectMiddlewareTest.php +++ b/module/Core/test/ShortUrl/Middleware/ExtraPathRedirectMiddlewareTest.php @@ -136,7 +136,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('longUrl'); + $shortUrl = ShortUrl::withLongUrl('https://longUrl'); $currentIteration = 1; $this->resolver->expects($this->exactly($expectedResolveCalls))->method('resolveEnabledShortUrl')->with( diff --git a/module/Core/test/ShortUrl/Model/ShortUrlCreationTest.php b/module/Core/test/ShortUrl/Model/ShortUrlCreationTest.php index a46474c0..bb82ee8e 100644 --- a/module/Core/test/ShortUrl/Model/ShortUrlCreationTest.php +++ b/module/Core/test/ShortUrl/Model/ShortUrlCreationTest.php @@ -33,37 +33,37 @@ class ShortUrlCreationTest extends TestCase { yield [[]]; yield [[ - ShortUrlInputFilter::LONG_URL => 'foo', + ShortUrlInputFilter::LONG_URL => 'https://foo', ShortUrlInputFilter::VALID_SINCE => '', ShortUrlInputFilter::VALID_UNTIL => '', ShortUrlInputFilter::CUSTOM_SLUG => 'foobar', ShortUrlInputFilter::MAX_VISITS => 'invalid', ]]; yield [[ - ShortUrlInputFilter::LONG_URL => 'foo', + ShortUrlInputFilter::LONG_URL => 'https://foo', ShortUrlInputFilter::VALID_SINCE => '2017', ShortUrlInputFilter::MAX_VISITS => 5, ]]; yield [[ - ShortUrlInputFilter::LONG_URL => 'foo', + ShortUrlInputFilter::LONG_URL => 'https://foo', ShortUrlInputFilter::VALID_SINCE => new stdClass(), ShortUrlInputFilter::VALID_UNTIL => 'foo', ]]; yield [[ - ShortUrlInputFilter::LONG_URL => 'foo', + ShortUrlInputFilter::LONG_URL => 'https://foo', ShortUrlInputFilter::VALID_UNTIL => 500, ShortUrlInputFilter::DOMAIN => 4, ]]; yield [[ - ShortUrlInputFilter::LONG_URL => 'foo', + ShortUrlInputFilter::LONG_URL => 'https://foo', ShortUrlInputFilter::SHORT_CODE_LENGTH => 3, ]]; yield [[ - ShortUrlInputFilter::LONG_URL => 'foo', + ShortUrlInputFilter::LONG_URL => 'https://foo', ShortUrlInputFilter::CUSTOM_SLUG => '', ]]; yield [[ - ShortUrlInputFilter::LONG_URL => 'foo', + ShortUrlInputFilter::LONG_URL => 'https://foo', ShortUrlInputFilter::CUSTOM_SLUG => ' ', ]]; yield [[ @@ -73,33 +73,42 @@ class ShortUrlCreationTest extends TestCase ShortUrlInputFilter::LONG_URL => null, ]]; yield [[ - ShortUrlInputFilter::LONG_URL => 'foo', + ShortUrlInputFilter::LONG_URL => 'missing_schema', + ]]; + yield [[ + ShortUrlInputFilter::LONG_URL => 'https://foo', ShortUrlInputFilter::DEVICE_LONG_URLS => [ 'invalid' => 'https://shlink.io', ], ]]; yield [[ - ShortUrlInputFilter::LONG_URL => 'foo', + ShortUrlInputFilter::LONG_URL => 'https://foo', ShortUrlInputFilter::DEVICE_LONG_URLS => [ DeviceType::DESKTOP->value => '', ], ]]; yield [[ - ShortUrlInputFilter::LONG_URL => 'foo', + ShortUrlInputFilter::LONG_URL => 'https://foo', ShortUrlInputFilter::DEVICE_LONG_URLS => [ DeviceType::DESKTOP->value => null, ], ]]; yield [[ - ShortUrlInputFilter::LONG_URL => 'foo', + ShortUrlInputFilter::LONG_URL => 'https://foo', ShortUrlInputFilter::DEVICE_LONG_URLS => [ DeviceType::IOS->value => ' ', ], ]]; yield [[ - ShortUrlInputFilter::LONG_URL => 'foo', + ShortUrlInputFilter::LONG_URL => 'https://foo', ShortUrlInputFilter::DEVICE_LONG_URLS => [ - DeviceType::IOS->value => 'bar', + DeviceType::ANDROID->value => 'missing_schema', + ], + ]]; + yield [[ + ShortUrlInputFilter::LONG_URL => 'https://foo', + ShortUrlInputFilter::DEVICE_LONG_URLS => [ + DeviceType::IOS->value => 'https://bar', DeviceType::ANDROID->value => [], ], ]]; @@ -115,7 +124,7 @@ class ShortUrlCreationTest extends TestCase $creation = ShortUrlCreation::fromRawData([ 'validSince' => Chronos::parse('2015-01-01')->toAtomString(), 'customSlug' => $customSlug, - 'longUrl' => 'longUrl', + 'longUrl' => 'https://longUrl', ], new UrlShortenerOptions(multiSegmentSlugsEnabled: $multiSegmentEnabled, mode: $shortUrlMode)); self::assertTrue($creation->hasValidSince()); @@ -161,7 +170,7 @@ class ShortUrlCreationTest extends TestCase { $creation = ShortUrlCreation::fromRawData([ 'title' => $title, - 'longUrl' => 'longUrl', + 'longUrl' => 'https://longUrl', ]); self::assertEquals($expectedTitle, $creation->title); @@ -184,7 +193,7 @@ class ShortUrlCreationTest extends TestCase { $creation = ShortUrlCreation::fromRawData([ 'domain' => $domain, - 'longUrl' => 'longUrl', + 'longUrl' => 'https://longUrl', ]); self::assertSame($expectedDomain, $creation->domain); diff --git a/module/Core/test/ShortUrl/Model/ShortUrlEditionTest.php b/module/Core/test/ShortUrl/Model/ShortUrlEditionTest.php index 720c290f..5d77d806 100644 --- a/module/Core/test/ShortUrl/Model/ShortUrlEditionTest.php +++ b/module/Core/test/ShortUrl/Model/ShortUrlEditionTest.php @@ -31,23 +31,29 @@ class ShortUrlEditionTest extends TestCase yield 'null' => [null, [], []]; yield 'empty' => [[], [], []]; yield 'only new urls' => [[ - DeviceType::DESKTOP->value => 'foo', - DeviceType::IOS->value => 'bar', + DeviceType::DESKTOP->value => 'https://foo', + DeviceType::IOS->value => 'https://bar', ], [ - DeviceType::DESKTOP->value => DeviceLongUrlPair::fromRawTypeAndLongUrl(DeviceType::DESKTOP->value, 'foo'), - DeviceType::IOS->value => DeviceLongUrlPair::fromRawTypeAndLongUrl(DeviceType::IOS->value, 'bar'), + DeviceType::DESKTOP->value => DeviceLongUrlPair::fromRawTypeAndLongUrl( + DeviceType::DESKTOP->value, + 'https://foo', + ), + DeviceType::IOS->value => DeviceLongUrlPair::fromRawTypeAndLongUrl(DeviceType::IOS->value, 'https://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::DESKTOP->value => 'https://bar', + DeviceType::IOS->value => 'https://foo', DeviceType::ANDROID->value => null, ], [ - DeviceType::DESKTOP->value => DeviceLongUrlPair::fromRawTypeAndLongUrl(DeviceType::DESKTOP->value, 'bar'), - DeviceType::IOS->value => DeviceLongUrlPair::fromRawTypeAndLongUrl(DeviceType::IOS->value, 'foo'), + DeviceType::DESKTOP->value => DeviceLongUrlPair::fromRawTypeAndLongUrl( + DeviceType::DESKTOP->value, + 'https://bar', + ), + DeviceType::IOS->value => DeviceLongUrlPair::fromRawTypeAndLongUrl(DeviceType::IOS->value, 'https://foo'), ], [DeviceType::ANDROID]]; } } diff --git a/module/Core/test/ShortUrl/ShortUrlResolverTest.php b/module/Core/test/ShortUrl/ShortUrlResolverTest.php index 0ecafd3a..f2b89586 100644 --- a/module/Core/test/ShortUrl/ShortUrlResolverTest.php +++ b/module/Core/test/ShortUrl/ShortUrlResolverTest.php @@ -45,7 +45,7 @@ class ShortUrlResolverTest extends TestCase #[Test, DataProvider('provideAdminApiKeys')] public function shortCodeIsProperlyParsed(?ApiKey $apiKey): void { - $shortUrl = ShortUrl::withLongUrl('expected_url'); + $shortUrl = ShortUrl::withLongUrl('https://expected_url'); $shortCode = $shortUrl->getShortCode(); $identifier = ShortUrlIdentifier::fromShortCodeAndDomain($shortCode); @@ -76,7 +76,7 @@ class ShortUrlResolverTest extends TestCase #[Test] public function shortCodeToEnabledShortUrlProperlyParsesShortCode(): void { - $shortUrl = ShortUrl::withLongUrl('expected_url'); + $shortUrl = ShortUrl::withLongUrl('https://expected_url'); $shortCode = $shortUrl->getShortCode(); $this->repo->expects($this->once())->method('findOneWithDomainFallback')->with( @@ -111,7 +111,9 @@ class ShortUrlResolverTest extends TestCase $now = Chronos::now(); yield 'maxVisits reached' => [(function () { - $shortUrl = ShortUrl::create(ShortUrlCreation::fromRawData(['maxVisits' => 3, 'longUrl' => 'longUrl'])); + $shortUrl = ShortUrl::create( + ShortUrlCreation::fromRawData(['maxVisits' => 3, 'longUrl' => 'https://longUrl']), + ); $shortUrl->setVisits(new ArrayCollection(map( range(0, 4), fn () => Visit::forValidShortUrl($shortUrl, Visitor::emptyInstance()), @@ -120,16 +122,16 @@ class ShortUrlResolverTest extends TestCase return $shortUrl; })()]; yield 'future validSince' => [ShortUrl::create(ShortUrlCreation::fromRawData( - ['validSince' => $now->addMonth()->toAtomString(), 'longUrl' => 'longUrl'], + ['validSince' => $now->addMonth()->toAtomString(), 'longUrl' => 'https://longUrl'], ))]; yield 'past validUntil' => [ShortUrl::create(ShortUrlCreation::fromRawData( - ['validUntil' => $now->subMonth()->toAtomString(), 'longUrl' => 'longUrl'], + ['validUntil' => $now->subMonth()->toAtomString(), 'longUrl' => 'https://longUrl'], ))]; yield 'mixed' => [(function () use ($now) { $shortUrl = ShortUrl::create(ShortUrlCreation::fromRawData([ 'maxVisits' => 3, 'validUntil' => $now->subMonth()->toAtomString(), - 'longUrl' => 'longUrl', + 'longUrl' => 'https://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 60959215..409c937f 100644 --- a/module/Core/test/ShortUrl/ShortUrlServiceTest.php +++ b/module/Core/test/ShortUrl/ShortUrlServiceTest.php @@ -57,7 +57,7 @@ class ShortUrlServiceTest extends TestCase ShortUrlEdition $shortUrlEdit, ?ApiKey $apiKey, ): void { - $originalLongUrl = 'originalLongUrl'; + $originalLongUrl = 'https://originalLongUrl'; $shortUrl = ShortUrl::withLongUrl($originalLongUrl); $this->urlResolver->expects($this->once())->method('resolveShortUrl')->with( @@ -103,16 +103,16 @@ class ShortUrlServiceTest extends TestCase yield 'long URL and API key' => [new InvokedCount(1), ShortUrlEdition::fromRawData([ 'validSince' => Chronos::parse('2017-01-01 00:00:00')->toAtomString(), 'maxVisits' => 10, - 'longUrl' => 'modifiedLongUrl', + 'longUrl' => 'https://modifiedLongUrl', ]), ApiKey::create()]; yield 'long URL with validation' => [new InvokedCount(1), ShortUrlEdition::fromRawData([ - 'longUrl' => 'modifiedLongUrl', + 'longUrl' => 'https://modifiedLongUrl', 'validateUrl' => true, ]), null]; yield 'device redirects' => [new InvokedCount(0), ShortUrlEdition::fromRawData([ 'deviceLongUrls' => [ - DeviceType::IOS->value => 'iosLongUrl', - DeviceType::ANDROID->value => 'androidLongUrl', + DeviceType::IOS->value => 'https://iosLongUrl', + DeviceType::ANDROID->value => 'https://androidLongUrl', ], ]), null]; } diff --git a/module/Core/test/ShortUrl/Transformer/ShortUrlDataTransformerTest.php b/module/Core/test/ShortUrl/Transformer/ShortUrlDataTransformerTest.php index 34474ea3..27916063 100644 --- a/module/Core/test/ShortUrl/Transformer/ShortUrlDataTransformerTest.php +++ b/module/Core/test/ShortUrl/Transformer/ShortUrlDataTransformerTest.php @@ -44,7 +44,7 @@ class ShortUrlDataTransformerTest extends TestCase ]]; yield 'max visits only' => [ShortUrl::create(ShortUrlCreation::fromRawData([ 'maxVisits' => $maxVisits, - 'longUrl' => 'longUrl', + 'longUrl' => 'https://longUrl', ])), [ 'validSince' => null, 'validUntil' => null, @@ -52,7 +52,7 @@ class ShortUrlDataTransformerTest extends TestCase ]]; yield 'max visits and valid since' => [ ShortUrl::create(ShortUrlCreation::fromRawData( - ['validSince' => $now, 'maxVisits' => $maxVisits, 'longUrl' => 'longUrl'], + ['validSince' => $now, 'maxVisits' => $maxVisits, 'longUrl' => 'https://longUrl'], )), [ 'validSince' => $now->toAtomString(), @@ -62,7 +62,7 @@ class ShortUrlDataTransformerTest extends TestCase ]; yield 'both dates' => [ ShortUrl::create(ShortUrlCreation::fromRawData( - ['validSince' => $now, 'validUntil' => $now->subDays(10), 'longUrl' => 'longUrl'], + ['validSince' => $now, 'validUntil' => $now->subDays(10), 'longUrl' => 'https://longUrl'], )), [ 'validSince' => $now->toAtomString(), @@ -75,7 +75,7 @@ class ShortUrlDataTransformerTest extends TestCase 'validSince' => $now, 'validUntil' => $now->subDays(5), 'maxVisits' => $maxVisits, - 'longUrl' => 'longUrl', + 'longUrl' => 'https://longUrl', ])), [ 'validSince' => $now->toAtomString(), diff --git a/module/Core/test/Visit/Geolocation/VisitLocatorTest.php b/module/Core/test/Visit/Geolocation/VisitLocatorTest.php index 4621f1a7..70fc6243 100644 --- a/module/Core/test/Visit/Geolocation/VisitLocatorTest.php +++ b/module/Core/test/Visit/Geolocation/VisitLocatorTest.php @@ -47,8 +47,10 @@ class VisitLocatorTest extends TestCase ): void { $unlocatedVisits = map( range(1, 200), - fn (int $i) => - Visit::forValidShortUrl(ShortUrl::withLongUrl(sprintf('short_code_%s', $i)), Visitor::emptyInstance()), + fn (int $i) => Visit::forValidShortUrl( + ShortUrl::withLongUrl(sprintf('https://short_code_%s', $i)), + Visitor::emptyInstance(), + ), ); $this->repo->expects($this->once())->method($expectedRepoMethodName)->willReturn($unlocatedVisits); @@ -85,7 +87,7 @@ class VisitLocatorTest extends TestCase bool $isNonLocatableAddress, ): void { $unlocatedVisits = [ - Visit::forValidShortUrl(ShortUrl::withLongUrl('foo'), Visitor::emptyInstance()), + Visit::forValidShortUrl(ShortUrl::withLongUrl('https://foo'), Visitor::emptyInstance()), ]; $this->repo->expects($this->once())->method($expectedRepoMethodName)->willReturn($unlocatedVisits); diff --git a/module/Rest/src/ApiKey/Spec/WithApiKeySpecsEnsuringJoin.php b/module/Rest/src/ApiKey/Spec/WithApiKeySpecsEnsuringJoin.php index 1f8c2fd3..9a8f8056 100644 --- a/module/Rest/src/ApiKey/Spec/WithApiKeySpecsEnsuringJoin.php +++ b/module/Rest/src/ApiKey/Spec/WithApiKeySpecsEnsuringJoin.php @@ -11,14 +11,14 @@ use Shlinkio\Shlink\Rest\Entity\ApiKey; class WithApiKeySpecsEnsuringJoin extends BaseSpecification { - public function __construct(private ?ApiKey $apiKey, private string $fieldToJoin = 'shortUrls') + public function __construct(private readonly ?ApiKey $apiKey, private readonly string $fieldToJoin = 'shortUrls') { parent::__construct(); } protected function getSpec(): Specification { - return $this->apiKey === null || $this->apiKey->isAdmin() ? Spec::andX() : Spec::andX( + return $this->apiKey === null || ApiKey::isAdmin($this->apiKey) ? Spec::andX() : Spec::andX( Spec::join($this->fieldToJoin, 's'), $this->apiKey->spec($this->fieldToJoin), ); diff --git a/module/Rest/src/ApiKey/Spec/WithInlinedApiKeySpecsEnsuringJoin.php b/module/Rest/src/ApiKey/Spec/WithInlinedApiKeySpecsEnsuringJoin.php deleted file mode 100644 index 2e84d835..00000000 --- a/module/Rest/src/ApiKey/Spec/WithInlinedApiKeySpecsEnsuringJoin.php +++ /dev/null @@ -1,26 +0,0 @@ -apiKey === null || $this->apiKey->isAdmin() ? Spec::andX() : Spec::andX( - Spec::join($this->fieldToJoin, 's'), - $this->apiKey->inlinedSpec(), - ); - } -} diff --git a/module/Rest/src/Entity/ApiKey.php b/module/Rest/src/Entity/ApiKey.php index 57fecdd0..88cfa27e 100644 --- a/module/Rest/src/Entity/ApiKey.php +++ b/module/Rest/src/Entity/ApiKey.php @@ -114,9 +114,12 @@ class ApiKey extends AbstractEntity return Spec::andX(...$specs); } - public function isAdmin(): bool + /** + * @return ($apiKey is null ? true : boolean) + */ + public static function isAdmin(?ApiKey $apiKey): bool { - return $this->roles->isEmpty(); + return $apiKey === null || $apiKey->roles->isEmpty(); } public function hasRole(Role $role): bool diff --git a/module/Rest/test-api/Action/CreateShortUrlTest.php b/module/Rest/test-api/Action/CreateShortUrlTest.php index d93fc9f1..108a0f6f 100644 --- a/module/Rest/test-api/Action/CreateShortUrlTest.php +++ b/module/Rest/test-api/Action/CreateShortUrlTest.php @@ -268,6 +268,8 @@ class CreateShortUrlTest extends ApiTestCase 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 'missing url schema v2' => [['longUrl' => 'foo.com'], '2', 'INVALID_ARGUMENT']; + yield 'missing url schema v3' => [['longUrl' => 'foo.com'], '3', 'https://shlink.io/api/error/invalid-data']; yield 'empty device long url v2' => [[ 'longUrl' => 'foo', 'deviceLongUrls' => [ diff --git a/module/Rest/test-api/Action/EditShortUrlTest.php b/module/Rest/test-api/Action/EditShortUrlTest.php index 8ac26a27..22833970 100644 --- a/module/Rest/test-api/Action/EditShortUrlTest.php +++ b/module/Rest/test-api/Action/EditShortUrlTest.php @@ -96,7 +96,7 @@ class EditShortUrlTest extends ApiTestCase public static function provideLongUrls(): iterable { yield 'valid URL' => ['https://shlink.io', self::STATUS_OK, null]; - yield 'invalid URL' => ['htt:foo', self::STATUS_BAD_REQUEST, 'INVALID_URL']; + yield 'invalid URL' => ['http://foo', self::STATUS_BAD_REQUEST, 'INVALID_URL']; } #[Test, DataProvider('provideInvalidUrls')]