diff --git a/.github/actions/ci-setup/action.yml b/.github/actions/ci-setup/action.yml index b45fa6c2..3a6a8642 100644 --- a/.github/actions/ci-setup/action.yml +++ b/.github/actions/ci-setup/action.yml @@ -43,5 +43,5 @@ runs: coverage: xdebug - name: Install dependencies if: ${{ inputs.install-deps == 'yes' }} - run: composer install --no-interaction --prefer-dist ${{ inputs.php-version == '8.5' && '--ignore-platform-req=php' || '' }} + run: composer install --no-interaction --prefer-dist shell: bash diff --git a/.github/workflows/ci-db-tests.yml b/.github/workflows/ci-db-tests.yml index 28ec4fd6..639481b8 100644 --- a/.github/workflows/ci-db-tests.yml +++ b/.github/workflows/ci-db-tests.yml @@ -14,7 +14,6 @@ jobs: strategy: matrix: php-version: ['8.3', '8.4', '8.5'] - continue-on-error: ${{ matrix.php-version == '8.5' }} env: LC_ALL: C steps: diff --git a/.github/workflows/ci-tests.yml b/.github/workflows/ci-tests.yml index 1b196ac1..1ee23377 100644 --- a/.github/workflows/ci-tests.yml +++ b/.github/workflows/ci-tests.yml @@ -14,7 +14,6 @@ jobs: strategy: matrix: php-version: ['8.3', '8.4', '8.5'] - continue-on-error: ${{ matrix.php-version == '8.5' }} env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} # rr get-binary picks this env automatically steps: diff --git a/.github/workflows/publish-release.yml b/.github/workflows/publish-release.yml index a6b923b4..61fc6940 100644 --- a/.github/workflows/publish-release.yml +++ b/.github/workflows/publish-release.yml @@ -10,7 +10,7 @@ jobs: runs-on: ubuntu-24.04 strategy: matrix: - php-version: ['8.3', '8.4'] + php-version: ['8.3', '8.4', '8.5'] steps: - uses: actions/checkout@v4 - uses: './.github/actions/ci-setup' diff --git a/CHANGELOG.md b/CHANGELOG.md index 6bc198d6..c5a3a245 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,45 @@ 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). +## [4.6.0] - 2025-11-01 +### Added +* [#2327](https://github.com/shlinkio/shlink/issues/2327) Allow filtering short URL lists by those not including certain tags. + + Now, the `GET /short-urls` endpoint accepts two new params: `excludeTags`, which is an array of strings with the tags that should not be included, and `excludeTagsMode`, which accepts the values `any` and `all`, and determines if short URLs should be filtered out if they contain any of the excluded tags, or all the excluded tags. + + Additionally, the `short-url:list` command also supports the same feature via `--exclude-tag` option, which requires a value and can be provided multiple times, and `--exclude-tags-all`, which does not expect a value and determines if the mode should be `all`, or `any`. + +* [#2192](https://github.com/shlinkio/shlink/issues/2192) Allow filtering short URL lists by the API key that was used to create them. + + Now, the `GET /short-urls` endpoint accepts a new `apiKeyName` param, which is ignored if the request is performed with a non-admin API key which name does not match the one provided here. + + Additionally, the `short-url:list` command also supports the same feature via the `--api-key-name` option. + +* [#2330](https://github.com/shlinkio/shlink/issues/2330) Add support to serve Shlink with FrankenPHP, by providing a worker script in `bin/frankenphp-worker.php`. + +* [#2449](https://github.com/shlinkio/shlink/issues/2449) Add support to provide redis credentials separately when using redis sentinels, where provided servers are the sentinels and not the redis instances. + + For this, Shlink supports two new env ras / config options, as `REDIS_SERVERS_USER` and `REDIS_SERVERS_PASSWORD`. + +* [#2498](https://github.com/shlinkio/shlink/issues/2498) Allow orphan visits, non-orphan visits and tag visits lists to be filtered by domain. + + This is done via the `domain` query parameter in API endpoints, and via the `--domain` option in console commands. + +* [#2472](https://github.com/shlinkio/shlink/issues/2472) Add support for PHP 8.5 + +### Changed +* [#2424](https://github.com/shlinkio/shlink/issues/2424) Make simple console commands invokable. + +### Deprecated +* *Nothing* + +### Removed +* *Nothing* + +### Fixed +* *Nothing* + + ## [4.5.3] - 2025-10-10 ### Added * *Nothing* diff --git a/README.md b/README.md index e061467d..dc23d7f6 100644 --- a/README.md +++ b/README.md @@ -99,6 +99,12 @@ Both the API and CLI allow you to do mostly the same operations, except for API If you are trying to find out how to run the project in development mode or how to provide contributions, read the [CONTRIBUTING](CONTRIBUTING.md) doc. +## Powered by + +Thanks to [JetBrains](https://www.jetbrains.com/) for their continuous support to this project in the form of IDE licenses. + +![JetBrains logo](https://resources.jetbrains.com/storage/products/company/brand/logos/jetbrains.svg) + --- > This product includes GeoLite2 data created by MaxMind, available from [https://www.maxmind.com](https://www.maxmind.com) diff --git a/composer.json b/composer.json index f04124d4..dc851009 100644 --- a/composer.json +++ b/composer.json @@ -20,9 +20,9 @@ "ext-pdo": "*", "akrabat/ip-address-middleware": "^2.6", "cakephp/chronos": "^3.1", - "doctrine/dbal": "^4.2", - "doctrine/migrations": "^3.8", - "doctrine/orm": "^3.3", + "doctrine/dbal": "^4.3", + "doctrine/migrations": "^3.9", + "doctrine/orm": "^3.5", "donatj/phpuseragentparser": "^1.10", "endroid/qr-code": "^6.0.5", "friendsofphp/proxy-manager-lts": "^1.0", @@ -43,11 +43,11 @@ "pagerfanta/core": "^3.8", "ramsey/uuid": "^4.7", "shlinkio/doctrine-specification": "^2.2", - "shlinkio/shlink-common": "^7.1", + "shlinkio/shlink-common": "^7.2", "shlinkio/shlink-config": "^4.0", "shlinkio/shlink-event-dispatcher": "^4.3", "shlinkio/shlink-importer": "^5.6", - "shlinkio/shlink-installer": "^9.6", + "shlinkio/shlink-installer": "^9.7", "shlinkio/shlink-ip-geolocation": "^4.4", "shlinkio/shlink-json": "^1.2", "spiral/roadrunner": "^2025.1", diff --git a/config/autoload/cache.global.php b/config/autoload/cache.global.php index 6ae37de7..670a6a61 100644 --- a/config/autoload/cache.global.php +++ b/config/autoload/cache.global.php @@ -11,6 +11,8 @@ return (static function (): array { 'redis' => [ 'servers' => $redisServers, 'sentinel_service' => EnvVars::REDIS_SENTINEL_SERVICE->loadFromEnv(), + 'username' => EnvVars::REDIS_SERVERS_USER->loadFromEnv(), + 'password' => EnvVars::REDIS_SERVERS_PASSWORD->loadFromEnv(), ], ]; diff --git a/config/autoload/installer.global.php b/config/autoload/installer.global.php index 7f9e8900..a3918aff 100644 --- a/config/autoload/installer.global.php +++ b/config/autoload/installer.global.php @@ -33,6 +33,8 @@ return [ Option\Cache\CacheNamespaceConfigOption::class, Option\Redis\RedisServersConfigOption::class, Option\Redis\RedisSentinelServiceConfigOption::class, + Option\Redis\RedisServersUserConfigOption::class, + Option\Redis\RedisServersPasswordConfigOption::class, Option\Redis\RedisPubSubConfigOption::class, Option\UrlShortener\ShortCodeLengthOption::class, Option\Mercure\EnableMercureConfigOption::class, diff --git a/docs/swagger/paths/v1_short-urls.json b/docs/swagger/paths/v1_short-urls.json index 6ca05c2e..1a58ef14 100644 --- a/docs/swagger/paths/v1_short-urls.json +++ b/docs/swagger/paths/v1_short-urls.json @@ -31,7 +31,7 @@ { "name": "searchTerm", "in": "query", - "description": "A query used to filter results by searching for it on the longUrl and shortCode fields. (Since v1.3.0)", + "description": "A query used to filter results by searching for it on the longUrl and shortCode fields.", "required": false, "schema": { "type": "string" @@ -40,7 +40,7 @@ { "name": "tags[]", "in": "query", - "description": "A list of tags used to filter the result set. Only short URLs tagged with at least one of the provided tags will be returned. (Since v1.3.0)", + "description": "A list of tags used to filter the result set. Only short URLs **with** these tags will be returned.", "required": false, "schema": { "type": "array", @@ -52,7 +52,29 @@ { "name": "tagsMode", "in": "query", - "description": "Tells how the filtering by tags should work, returning short URLs containing \"any\" of the tags, or \"all\" the tags. It's ignored if no tags are provided, and defaults to \"any\" if not provided.", + "description": "Tells how the filtering by `tags` should work, returning short URLs containing \"any\" of the tags, or \"all\" the tags. Defaults to \"any\".
It's ignored if `tags` is not provided.", + "required": false, + "schema": { + "type": "string", + "enum": ["any", "all"] + } + }, + { + "name": "excludeTags[]", + "in": "query", + "description": "A list of tags used to filter the result set. Only short URLs **without** these tags will be returned.", + "required": false, + "schema": { + "type": "array", + "items": { + "type": "string" + } + } + }, + { + "name": "excludeTagsMode", + "in": "query", + "description": "Tells how the filtering by `excludeTags` should work, returning short URLs not containing \"any\" of the tags, or not containing \"all\" the tags. Defaults to \"any\".
It's ignored if `excludeTags` is not provided.", "required": false, "schema": { "type": "string", @@ -134,6 +156,15 @@ "schema": { "type": "string" } + }, + { + "name": "apiKeyName", + "in": "query", + "description": "Only get short URLs created with this API key.
This value is **ignored** if the request is performed with a non-admin API key that does not match this name.", + "required": false, + "schema": { + "type": "string" + } } ], "security": [ diff --git a/docs/swagger/paths/v2_tags_{tag}_visits.json b/docs/swagger/paths/v2_tags_{tag}_visits.json index 1f3dabf2..70cfc6e2 100644 --- a/docs/swagger/paths/v2_tags_{tag}_visits.json +++ b/docs/swagger/paths/v2_tags_{tag}_visits.json @@ -64,6 +64,10 @@ "type": "string", "enum": ["true"] } + }, + { + "$ref": "../parameters/domain.json", + "description": "Return visits for short URLs that belong to this domain. Use **DEFAULT** keyword to return visits from default domain." } ], "security": [ diff --git a/docs/swagger/paths/v2_visits_non-orphan.json b/docs/swagger/paths/v2_visits_non-orphan.json index 65b11252..3579030a 100644 --- a/docs/swagger/paths/v2_visits_non-orphan.json +++ b/docs/swagger/paths/v2_visits_non-orphan.json @@ -55,6 +55,10 @@ "type": "string", "enum": ["true"] } + }, + { + "$ref": "../parameters/domain.json", + "description": "Return visits for short URLs that belong to this domain. Use **DEFAULT** keyword to return visits from default domain." } ], "security": [ diff --git a/docs/swagger/paths/v2_visits_orphan.json b/docs/swagger/paths/v2_visits_orphan.json index df2ee0cd..1653e36a 100644 --- a/docs/swagger/paths/v2_visits_orphan.json +++ b/docs/swagger/paths/v2_visits_orphan.json @@ -65,6 +65,10 @@ "type": "string", "enum": ["invalid_short_url", "base_url", "regular_404"] } + }, + { + "$ref": "../parameters/domain.json", + "description": "Return only visits for this domain. Use **DEFAULT** keyword to return visits from default domain." } ], "security": [ diff --git a/module/CLI/config/cli.config.php b/module/CLI/config/cli.config.php index a554db40..669a3788 100644 --- a/module/CLI/config/cli.config.php +++ b/module/CLI/config/cli.config.php @@ -26,6 +26,7 @@ return [ Command\Api\GenerateKeyCommand::NAME => Command\Api\GenerateKeyCommand::class, Command\Api\DisableKeyCommand::NAME => Command\Api\DisableKeyCommand::class, + Command\Api\DeleteKeyCommand::NAME => Command\Api\DeleteKeyCommand::class, Command\Api\ListKeysCommand::NAME => Command\Api\ListKeysCommand::class, Command\Api\InitialApiKeyCommand::NAME => Command\Api\InitialApiKeyCommand::class, Command\Api\RenameApiKeyCommand::NAME => Command\Api\RenameApiKeyCommand::class, diff --git a/module/CLI/config/dependencies.config.php b/module/CLI/config/dependencies.config.php index 5df74822..b4bf15d2 100644 --- a/module/CLI/config/dependencies.config.php +++ b/module/CLI/config/dependencies.config.php @@ -52,6 +52,7 @@ return [ Command\Api\GenerateKeyCommand::class => ConfigAbstractFactory::class, Command\Api\DisableKeyCommand::class => ConfigAbstractFactory::class, + Command\Api\DeleteKeyCommand::class => ConfigAbstractFactory::class, Command\Api\ListKeysCommand::class => ConfigAbstractFactory::class, Command\Api\InitialApiKeyCommand::class => ConfigAbstractFactory::class, Command\Api\RenameApiKeyCommand::class => ConfigAbstractFactory::class, @@ -108,6 +109,7 @@ return [ Command\Api\GenerateKeyCommand::class => [ApiKeyService::class, ApiKey\RoleResolver::class], Command\Api\DisableKeyCommand::class => [ApiKeyService::class], + Command\Api\DeleteKeyCommand::class => [ApiKeyService::class], Command\Api\ListKeysCommand::class => [ApiKeyService::class], Command\Api\InitialApiKeyCommand::class => [ApiKeyService::class], Command\Api\RenameApiKeyCommand::class => [ApiKeyService::class], diff --git a/module/CLI/src/Command/Api/DeleteKeyCommand.php b/module/CLI/src/Command/Api/DeleteKeyCommand.php new file mode 100644 index 00000000..f57a5e4a --- /dev/null +++ b/module/CLI/src/Command/Api/DeleteKeyCommand.php @@ -0,0 +1,94 @@ +%command.name% command allows you to delete an existing API key via its name. + + If no arguments are provided, you will be prompted to select one of the existing API keys. + + %command.full_name% + + You can optionally pass the API key name to be disabled: + + %command.full_name% the_key_name + + HELP, +)] +class DeleteKeyCommand extends Command +{ + public const string NAME = 'api-key:delete'; + + public function __construct(private readonly ApiKeyServiceInterface $apiKeyService) + { + parent::__construct(); + } + + protected function interact(InputInterface $input, OutputInterface $output): void + { + $apiKeyName = $input->getArgument('name'); + + if ($apiKeyName === null) { + $apiKeys = $this->apiKeyService->listKeys(); + $name = (new SymfonyStyle($input, $output))->choice( + 'What API key do you want to delete?', + map($apiKeys, static fn (ApiKey $apiKey) => $apiKey->name), + ); + + $input->setArgument('name', $name); + } + } + + public function __invoke( + SymfonyStyle $io, + InputInterface $input, + #[Argument(description: 'The API key to delete.')] + string|null $name = null, + ): int { + if ($name === null) { + $io->warning('An API key name was not provided.'); + return Command::INVALID; + } + + if (! $this->shouldProceed($io, $input)) { + return Command::INVALID; + } + + try { + $this->apiKeyService->deleteByName($name); + $io->success(sprintf('API key "%s" properly deleted', $name)); + return Command::SUCCESS; + } catch (ApiKeyNotFoundException $e) { + $io->error($e->getMessage()); + return Command::FAILURE; + } + } + + private function shouldProceed(SymfonyStyle $io, InputInterface $input): bool + { + if (! $input->isInteractive()) { + return true; + } + + $io->warning('You are about to delete an API key. This action cannot be undone.'); + return $io->confirm('Are you sure you want to delete the API key?'); + } +} diff --git a/module/CLI/src/Command/Api/InitialApiKeyCommand.php b/module/CLI/src/Command/Api/InitialApiKeyCommand.php index 66968eb3..680135d8 100644 --- a/module/CLI/src/Command/Api/InitialApiKeyCommand.php +++ b/module/CLI/src/Command/Api/InitialApiKeyCommand.php @@ -5,11 +5,15 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI\Command\Api; use Shlinkio\Shlink\Rest\Service\ApiKeyServiceInterface; +use Symfony\Component\Console\Attribute\Argument; +use Symfony\Component\Console\Attribute\AsCommand; use Symfony\Component\Console\Command\Command; -use Symfony\Component\Console\Input\InputArgument; -use Symfony\Component\Console\Input\InputInterface; -use Symfony\Component\Console\Output\OutputInterface; +use Symfony\Component\Console\Style\SymfonyStyle; +#[AsCommand( + name: InitialApiKeyCommand::NAME, + description: 'Tries to create initial API key', +)] class InitialApiKeyCommand extends Command { public const string NAME = 'api-key:initial'; @@ -19,22 +23,14 @@ class InitialApiKeyCommand extends Command parent::__construct(); } - protected function configure(): void - { - $this - ->setHidden() - ->setName(self::NAME) - ->setDescription('Tries to create initial API key') - ->addArgument('apiKey', InputArgument::REQUIRED, 'The initial API to create'); - } + public function __invoke( + SymfonyStyle $io, + #[Argument('The initial API to create')] string $apiKey, + ): int { + $result = $this->apiKeyService->createInitial($apiKey); - protected function execute(InputInterface $input, OutputInterface $output): int - { - $key = $input->getArgument('apiKey'); - $result = $this->apiKeyService->createInitial($key); - - if ($result === null && $output->isVerbose()) { - $output->writeln('Other API keys already exist. Initial API key creation skipped.'); + if ($result === null && $io->isVerbose()) { + $io->writeln('Other API keys already exist. Initial API key creation skipped.'); } return Command::SUCCESS; diff --git a/module/CLI/src/Command/Config/ReadEnvVarCommand.php b/module/CLI/src/Command/Config/ReadEnvVarCommand.php index e1cef3fd..e3a38be6 100644 --- a/module/CLI/src/Command/Config/ReadEnvVarCommand.php +++ b/module/CLI/src/Command/Config/ReadEnvVarCommand.php @@ -6,9 +6,10 @@ namespace Shlinkio\Shlink\CLI\Command\Config; use Closure; use Shlinkio\Shlink\Core\Config\EnvVars; +use Symfony\Component\Console\Attribute\Argument; +use Symfony\Component\Console\Attribute\AsCommand; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Exception\InvalidArgumentException; -use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; use Symfony\Component\Console\Style\SymfonyStyle; @@ -18,6 +19,11 @@ use function Shlinkio\Shlink\Core\ArrayUtils\contains; use function Shlinkio\Shlink\Core\enumValues; use function sprintf; +#[AsCommand( + name: ReadEnvVarCommand::NAME, + description: 'Display current value for an env var', + hidden: true, +)] class ReadEnvVarCommand extends Command { public const string NAME = 'env-var:read'; @@ -31,19 +37,10 @@ class ReadEnvVarCommand extends Command parent::__construct(); } - protected function configure(): void - { - $this - ->setName(self::NAME) - ->setHidden() - ->setDescription('Display current value for an env var') - ->addArgument('envVar', InputArgument::REQUIRED, 'The env var to read'); - } - protected function interact(InputInterface $input, OutputInterface $output): void { $io = new SymfonyStyle($input, $output); - $envVar = $input->getArgument('envVar'); + $envVar = $input->getArgument('env-var'); $validEnvVars = enumValues(EnvVars::class); if ($envVar === null) { @@ -54,14 +51,14 @@ class ReadEnvVarCommand extends Command throw new InvalidArgumentException(sprintf('%s is not a valid Shlink environment variable', $envVar)); } - $input->setArgument('envVar', $envVar); + $input->setArgument('env-var', $envVar); } - protected function execute(InputInterface $input, OutputInterface $output): int - { - $envVar = $input->getArgument('envVar'); - $output->writeln(formatEnvVarValue(($this->loadEnvVar)($envVar))); - + public function __invoke( + SymfonyStyle $io, + #[Argument(description: 'The env var to read')] string $envVar, + ): int { + $io->writeln(formatEnvVarValue(($this->loadEnvVar)($envVar))); return Command::SUCCESS; } } diff --git a/module/CLI/src/Command/Domain/DomainRedirectsCommand.php b/module/CLI/src/Command/Domain/DomainRedirectsCommand.php index 4c2e4350..1e272c12 100644 --- a/module/CLI/src/Command/Domain/DomainRedirectsCommand.php +++ b/module/CLI/src/Command/Domain/DomainRedirectsCommand.php @@ -7,8 +7,9 @@ namespace Shlinkio\Shlink\CLI\Command\Domain; use Shlinkio\Shlink\Core\Config\NotFoundRedirects; use Shlinkio\Shlink\Core\Domain\DomainServiceInterface; use Shlinkio\Shlink\Core\Domain\Model\DomainItem; +use Symfony\Component\Console\Attribute\Argument; +use Symfony\Component\Console\Attribute\AsCommand; use Symfony\Component\Console\Command\Command; -use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; use Symfony\Component\Console\Style\SymfonyStyle; @@ -18,6 +19,10 @@ use function array_map; use function sprintf; use function str_contains; +#[AsCommand( + name: DomainRedirectsCommand::NAME, + description: 'Set specific "not found" redirects for individual domains.', +)] class DomainRedirectsCommand extends Command { public const string NAME = 'domain:redirects'; @@ -27,18 +32,6 @@ class DomainRedirectsCommand extends Command parent::__construct(); } - protected function configure(): void - { - $this - ->setName(self::NAME) - ->setDescription('Set specific "not found" redirects for individual domains.') - ->addArgument( - 'domain', - InputArgument::REQUIRED, - 'The domain authority to which you want to set the specific redirects', - ); - } - protected function interact(InputInterface $input, OutputInterface $output): void { /** @var string|null $domain */ @@ -67,10 +60,11 @@ class DomainRedirectsCommand extends Command $input->setArgument('domain', str_contains($selectedOption, 'New domain') ? $askNewDomain() : $selectedOption); } - protected function execute(InputInterface $input, OutputInterface $output): int - { - $io = new SymfonyStyle($input, $output); - $domainAuthority = $input->getArgument('domain'); + public function __invoke( + SymfonyStyle $io, + #[Argument('The domain authority to which you want to set the specific redirects', name: 'domain')] + string $domainAuthority, + ): int { $domain = $this->domainService->findByAuthority($domainAuthority); $ask = static function (string $message, string|null $current) use ($io): string|null { diff --git a/module/CLI/src/Command/Domain/ListDomainsCommand.php b/module/CLI/src/Command/Domain/ListDomainsCommand.php index 935d272e..a66d6d7e 100644 --- a/module/CLI/src/Command/Domain/ListDomainsCommand.php +++ b/module/CLI/src/Command/Domain/ListDomainsCommand.php @@ -8,13 +8,17 @@ use Shlinkio\Shlink\CLI\Util\ShlinkTable; use Shlinkio\Shlink\Core\Config\NotFoundRedirectConfigInterface; use Shlinkio\Shlink\Core\Domain\DomainServiceInterface; use Shlinkio\Shlink\Core\Domain\Model\DomainItem; +use Symfony\Component\Console\Attribute\AsCommand; +use Symfony\Component\Console\Attribute\Option; use Symfony\Component\Console\Command\Command; -use Symfony\Component\Console\Input\InputInterface; -use Symfony\Component\Console\Input\InputOption; -use Symfony\Component\Console\Output\OutputInterface; +use Symfony\Component\Console\Style\SymfonyStyle; use function array_map; +#[AsCommand( + name: ListDomainsCommand::NAME, + description: 'List all domains that have been ever used for some short URL', +)] class ListDomainsCommand extends Command { public const string NAME = 'domain:list'; @@ -24,25 +28,17 @@ class ListDomainsCommand extends Command parent::__construct(); } - protected function configure(): void - { - $this - ->setName(self::NAME) - ->setDescription('List all domains that have been ever used for some short URL') - ->addOption( - 'show-redirects', - 'r', - InputOption::VALUE_NONE, - 'Will display an extra column with the information of the "not found" redirects for every domain.', - ); - } - - protected function execute(InputInterface $input, OutputInterface $output): int - { + public function __invoke( + SymfonyStyle $io, + #[Option( + 'Will display an extra column with the information of the "not found" redirects for every domain.', + shortcut: 'r', + )] + bool $showRedirects = false, + ): int { $domains = $this->domainService->listDomains(); - $showRedirects = $input->getOption('show-redirects'); $commonFields = ['Domain', 'Is default']; - $table = $showRedirects ? ShlinkTable::withRowSeparators($output) : ShlinkTable::default($output); + $table = $showRedirects ? ShlinkTable::withRowSeparators($io) : ShlinkTable::default($io); $table->render( $showRedirects ? [...$commonFields, '"Not found" redirects'] : $commonFields, @@ -53,7 +49,7 @@ class ListDomainsCommand extends Command ? [ ...$commonValues, $this->notFoundRedirectsToString($domain->notFoundRedirectConfig), - ] + ] : $commonValues; }, $domains), ); diff --git a/module/CLI/src/Command/Integration/MatomoSendVisitsCommand.php b/module/CLI/src/Command/Integration/MatomoSendVisitsCommand.php index c1c22075..f5d8e84c 100644 --- a/module/CLI/src/Command/Integration/MatomoSendVisitsCommand.php +++ b/module/CLI/src/Command/Integration/MatomoSendVisitsCommand.php @@ -8,10 +8,10 @@ use Cake\Chronos\Chronos; use Shlinkio\Shlink\Core\Matomo\MatomoOptions; use Shlinkio\Shlink\Core\Matomo\MatomoVisitSenderInterface; use Shlinkio\Shlink\Core\Matomo\VisitSendingProgressTrackerInterface; +use Symfony\Component\Console\Attribute\AsCommand; +use Symfony\Component\Console\Attribute\Option; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputInterface; -use Symfony\Component\Console\Input\InputOption; -use Symfony\Component\Console\Output\OutputInterface; use Symfony\Component\Console\Style\SymfonyStyle; use Throwable; @@ -19,22 +19,9 @@ use function Shlinkio\Shlink\Common\buildDateRange; use function Shlinkio\Shlink\Core\dateRangeToHumanFriendly; use function sprintf; -class MatomoSendVisitsCommand extends Command implements VisitSendingProgressTrackerInterface -{ - public const string NAME = 'integration:matomo:send-visits'; - - private readonly bool $matomoEnabled; - private SymfonyStyle $io; - - public function __construct(MatomoOptions $matomoOptions, private readonly MatomoVisitSenderInterface $visitSender) - { - $this->matomoEnabled = $matomoOptions->enabled; - parent::__construct(); - } - - protected function configure(): void - { - $help = <<%command.name% --since 2022-01-01 --until 2022-12-31 - HELP; + HELP, +)] +class MatomoSendVisitsCommand extends Command implements VisitSendingProgressTrackerInterface +{ + public const string NAME = 'integration:matomo:send-visits'; - $this - ->setName(self::NAME) - ->setDescription(sprintf( - '%sSend existing visits to the configured matomo instance', - $this->matomoEnabled ? '' : '[MATOMO INTEGRATION DISABLED] ', - )) - ->setHelp($help) - ->addOption( - 'since', - 's', - InputOption::VALUE_REQUIRED, - 'Only visits created since this date, inclusively, will be sent to Matomo', - ) - ->addOption( - 'until', - 'u', - InputOption::VALUE_REQUIRED, - 'Only visits created until this date, inclusively, will be sent to Matomo', - ); + private readonly bool $matomoEnabled; + private SymfonyStyle $io; + + public function __construct(MatomoOptions $matomoOptions, private readonly MatomoVisitSenderInterface $visitSender) + { + $this->matomoEnabled = $matomoOptions->enabled; + parent::__construct(); } - protected function execute(InputInterface $input, OutputInterface $output): int + protected function configure(): void { - $this->io = new SymfonyStyle($input, $output); + $this->setDescription(sprintf( + '%sSend existing visits to the configured matomo instance', + $this->matomoEnabled ? '' : '[MATOMO INTEGRATION DISABLED] ', + )); + } + + public function __invoke( + SymfonyStyle $io, + InputInterface $input, + #[Option('Only visits created since this date, inclusively, will be sent to Matomo', shortcut: 's')] + string|null $since = null, + #[Option('Only visits created until this date, inclusively, will be sent to Matomo', shortcut: 'u')] + string|null $until = null, + ): int { + $this->io = $io; if (! $this->matomoEnabled) { $this->io->warning('Matomo integration is not enabled in this Shlink instance'); @@ -87,8 +80,6 @@ class MatomoSendVisitsCommand extends Command implements VisitSendingProgressTra } // TODO Validate provided date formats - $since = $input->getOption('since'); - $until = $input->getOption('until'); $dateRange = buildDateRange( startDate: $since !== null ? Chronos::parse($since) : null, endDate: $until !== null ? Chronos::parse($until) : null, diff --git a/module/CLI/src/Command/ShortUrl/DeleteExpiredShortUrlsCommand.php b/module/CLI/src/Command/ShortUrl/DeleteExpiredShortUrlsCommand.php index 2b2abd01..626ac136 100644 --- a/module/CLI/src/Command/ShortUrl/DeleteExpiredShortUrlsCommand.php +++ b/module/CLI/src/Command/ShortUrl/DeleteExpiredShortUrlsCommand.php @@ -6,14 +6,18 @@ namespace Shlinkio\Shlink\CLI\Command\ShortUrl; use Shlinkio\Shlink\Core\ShortUrl\DeleteShortUrlServiceInterface; use Shlinkio\Shlink\Core\ShortUrl\Model\ExpiredShortUrlsConditions; +use Symfony\Component\Console\Attribute\AsCommand; +use Symfony\Component\Console\Attribute\Option; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputInterface; -use Symfony\Component\Console\Input\InputOption; -use Symfony\Component\Console\Output\OutputInterface; use Symfony\Component\Console\Style\SymfonyStyle; use function sprintf; +#[AsCommand( + name: DeleteExpiredShortUrlsCommand::NAME, + description: 'Deletes all short URLs that are considered expired, because they have a validUntil date in the past', +)] class DeleteExpiredShortUrlsCommand extends Command { public const string NAME = 'short-url:delete-expired'; @@ -23,32 +27,17 @@ class DeleteExpiredShortUrlsCommand extends Command parent::__construct(); } - protected function configure(): void - { - $this - ->setName(self::NAME) - ->setDescription( - 'Deletes all short URLs that are considered expired, because they have a validUntil date in the past', - ) - ->addOption( - 'evaluate-max-visits', - mode: InputOption::VALUE_NONE, - description: 'Also take into consideration short URLs which have reached their max amount of visits.', - ) - ->addOption('force', 'f', InputOption::VALUE_NONE, 'Delete short URLs with no confirmation') - ->addOption( - 'dry-run', - mode: InputOption::VALUE_NONE, - description: 'Delete short URLs with no confirmation', - ); - } - - protected function execute(InputInterface $input, OutputInterface $output): int - { - $io = new SymfonyStyle($input, $output); - $force = $input->getOption('force') || ! $input->isInteractive(); - $dryRun = $input->getOption('dry-run'); - $conditions = new ExpiredShortUrlsConditions(maxVisitsReached: $input->getOption('evaluate-max-visits')); + public function __invoke( + SymfonyStyle $io, + InputInterface $input, + #[Option('Also take into consideration short URLs which have reached their max amount of visits.')] + bool $evaluateMaxVisits = false, + #[Option('Delete short URLs with no confirmation', shortcut: 'f')] bool $force = false, + #[Option('Only check how many short URLs would be affected, without actually deleting them')] + bool $dryRun = false, + ): int { + $conditions = new ExpiredShortUrlsConditions(maxVisitsReached: $evaluateMaxVisits); + $force = $force || ! $input->isInteractive(); if (! $force && ! $dryRun) { $io->warning([ @@ -69,6 +58,7 @@ class DeleteExpiredShortUrlsCommand extends Command $result = $this->deleteShortUrlService->deleteExpiredShortUrls($conditions); $io->success(sprintf('%s expired short URLs have been deleted', $result)); + return self::SUCCESS; } } diff --git a/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php b/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php index 5bdc4c81..d850e831 100644 --- a/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php +++ b/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php @@ -6,6 +6,7 @@ namespace Shlinkio\Shlink\CLI\Command\ShortUrl; use Shlinkio\Shlink\CLI\Input\EndDateOption; use Shlinkio\Shlink\CLI\Input\StartDateOption; +use Shlinkio\Shlink\CLI\Input\TagsOption; use Shlinkio\Shlink\CLI\Util\ShlinkTable; use Shlinkio\Shlink\Common\Paginator\Paginator; use Shlinkio\Shlink\Common\Paginator\Util\PagerfantaUtils; @@ -36,6 +37,7 @@ class ListShortUrlsCommand extends Command private readonly StartDateOption $startDateOption; private readonly EndDateOption $endDateOption; + private readonly TagsOption $tagsOption; public function __construct( private readonly ShortUrlListServiceInterface $shortUrlService, @@ -44,6 +46,7 @@ class ListShortUrlsCommand extends Command parent::__construct(); $this->startDateOption = new StartDateOption($this, 'short URLs'); $this->endDateOption = new EndDateOption($this, 'short URLs'); + $this->tagsOption = new TagsOption($this, 'A list of tags that short URLs need to include.'); } protected function configure(): void @@ -70,17 +73,22 @@ class ListShortUrlsCommand extends Command InputOption::VALUE_REQUIRED, 'Used to filter results by domain. Use DEFAULT keyword to filter by default domain', ) + ->addOption('including-all-tags', 'i', InputOption::VALUE_NONE, '[DEPRECATED] Use --tags-all instead') ->addOption( - 'tags', - 't', - InputOption::VALUE_REQUIRED, - 'A comma-separated list of tags to filter results.', + 'tags-all', + mode: InputOption::VALUE_NONE, + description: 'If --tags is provided, returns only short URLs including ALL of them', ) ->addOption( - 'including-all-tags', - 'i', - InputOption::VALUE_NONE, - 'If tags is provided, returns only short URLs having ALL tags.', + 'exclude-tag', + 'et', + InputOption::VALUE_REQUIRED | InputOption::VALUE_IS_ARRAY, + 'A list of tags that short URLs should not have.', + ) + ->addOption( + 'exclude-tags-all', + mode: InputOption::VALUE_NONE, + description: 'If --exclude-tag is provided, returns only short URLs not including ANY of them', ) ->addOption( 'exclude-max-visits-reached', @@ -101,6 +109,12 @@ class ListShortUrlsCommand extends Command 'The field from which you want to order by. ' . 'Define ordering dir by passing ASC or DESC after "-" or ",".', ) + ->addOption( + 'api-key-name', + 'kn', + InputOption::VALUE_REQUIRED, + 'List only short URLs created by the API key matching provided name.', + ) ->addOption( 'show-tags', null, @@ -134,33 +148,32 @@ class ListShortUrlsCommand extends Command $io = new SymfonyStyle($input, $output); $page = (int) $input->getOption('page'); - $searchTerm = $input->getOption('search-term'); - $domain = $input->getOption('domain'); - $tags = $input->getOption('tags'); - $tagsMode = $input->getOption('including-all-tags') === true ? TagsMode::ALL->value : TagsMode::ANY->value; - $tags = ! empty($tags) ? explode(',', $tags) : []; - $all = $input->getOption('all'); - $startDate = $this->startDateOption->get($input, $output); - $endDate = $this->endDateOption->get($input, $output); - $orderBy = $this->processOrderBy($input); - $columnsMap = $this->resolveColumnsMap($input); + $tagsMode = $input->getOption('tags-all') === true || $input->getOption('including-all-tags') === true + ? TagsMode::ALL->value + : TagsMode::ANY->value; + $excludeTagsMode = $input->getOption('exclude-tags-all') === true ? TagsMode::ALL->value : TagsMode::ANY->value; $data = [ - ShortUrlsParamsInputFilter::SEARCH_TERM => $searchTerm, - ShortUrlsParamsInputFilter::DOMAIN => $domain, - ShortUrlsParamsInputFilter::TAGS => $tags, + ShortUrlsParamsInputFilter::SEARCH_TERM => $input->getOption('search-term'), + ShortUrlsParamsInputFilter::DOMAIN => $input->getOption('domain'), + ShortUrlsParamsInputFilter::TAGS => $this->tagsOption->get($input), ShortUrlsParamsInputFilter::TAGS_MODE => $tagsMode, - ShortUrlsParamsInputFilter::ORDER_BY => $orderBy, - ShortUrlsParamsInputFilter::START_DATE => $startDate?->toAtomString(), - ShortUrlsParamsInputFilter::END_DATE => $endDate?->toAtomString(), + ShortUrlsParamsInputFilter::EXCLUDE_TAGS => $input->getOption('exclude-tag'), + ShortUrlsParamsInputFilter::EXCLUDE_TAGS_MODE => $excludeTagsMode, + ShortUrlsParamsInputFilter::ORDER_BY => $this->processOrderBy($input), + ShortUrlsParamsInputFilter::START_DATE => $this->startDateOption->get($input, $output)?->toAtomString(), + ShortUrlsParamsInputFilter::END_DATE => $this->endDateOption->get($input, $output)?->toAtomString(), ShortUrlsParamsInputFilter::EXCLUDE_MAX_VISITS_REACHED => $input->getOption('exclude-max-visits-reached'), ShortUrlsParamsInputFilter::EXCLUDE_PAST_VALID_UNTIL => $input->getOption('exclude-past-valid-until'), + ShortUrlsParamsInputFilter::API_KEY_NAME => $input->getOption('api-key-name'), ]; + $all = $input->getOption('all'); if ($all) { $data[ShortUrlsParamsInputFilter::ITEMS_PER_PAGE] = Paginator::ALL_ITEMS; } + $columnsMap = $this->resolveColumnsMap($input); do { $data[ShortUrlsParamsInputFilter::PAGE] = $page; $result = $this->renderPage($output, $columnsMap, ShortUrlsParams::fromRawData($data), $all); @@ -168,7 +181,7 @@ class ListShortUrlsCommand extends Command $continue = $result->hasNextPage() && $io->confirm( sprintf('Continue with page %s?', $page), - false, + default: false, ); } while ($continue); diff --git a/module/CLI/src/Command/Tag/DeleteTagsCommand.php b/module/CLI/src/Command/Tag/DeleteTagsCommand.php index 2022a9dc..301cba26 100644 --- a/module/CLI/src/Command/Tag/DeleteTagsCommand.php +++ b/module/CLI/src/Command/Tag/DeleteTagsCommand.php @@ -5,12 +5,12 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI\Command\Tag; use Shlinkio\Shlink\Core\Tag\TagServiceInterface; +use Symfony\Component\Console\Attribute\AsCommand; +use Symfony\Component\Console\Attribute\Option; use Symfony\Component\Console\Command\Command; -use Symfony\Component\Console\Input\InputInterface; -use Symfony\Component\Console\Input\InputOption; -use Symfony\Component\Console\Output\OutputInterface; use Symfony\Component\Console\Style\SymfonyStyle; +#[AsCommand(name: DeleteTagsCommand::NAME, description: 'Deletes one or more tags.')] class DeleteTagsCommand extends Command { public const string NAME = 'tag:delete'; @@ -20,24 +20,13 @@ class DeleteTagsCommand extends Command parent::__construct(); } - protected function configure(): void - { - $this - ->setName(self::NAME) - ->setDescription('Deletes one or more tags.') - ->addOption( - 'name', - 't', - InputOption::VALUE_REQUIRED | InputOption::VALUE_IS_ARRAY, - 'The name of the tags to delete', - ); - } - - protected function execute(InputInterface $input, OutputInterface $output): int - { - $io = new SymfonyStyle($input, $output); - $tagNames = $input->getOption('name'); - + /** + * @param string[] $tagNames + */ + public function __invoke( + SymfonyStyle $io, + #[Option('The name of the tags to delete', name: 'name', shortcut: 't')] array $tagNames = [], + ): int { if (empty($tagNames)) { $io->warning('You have to provide at least one tag name'); return self::INVALID; @@ -45,6 +34,7 @@ class DeleteTagsCommand extends Command $this->tagService->deleteTags($tagNames); $io->success('Tags properly deleted'); + return self::SUCCESS; } } diff --git a/module/CLI/src/Command/Tag/GetTagVisitsCommand.php b/module/CLI/src/Command/Tag/GetTagVisitsCommand.php index b3c083bc..bac12ac2 100644 --- a/module/CLI/src/Command/Tag/GetTagVisitsCommand.php +++ b/module/CLI/src/Command/Tag/GetTagVisitsCommand.php @@ -5,24 +5,34 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI\Command\Tag; use Shlinkio\Shlink\CLI\Command\Visit\AbstractVisitsListCommand; +use Shlinkio\Shlink\CLI\Input\DomainOption; use Shlinkio\Shlink\Common\Paginator\Paginator; use Shlinkio\Shlink\Common\Util\DateRange; +use Shlinkio\Shlink\Core\Domain\Entity\Domain; use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlStringifierInterface; use Shlinkio\Shlink\Core\Visit\Entity\Visit; -use Shlinkio\Shlink\Core\Visit\Model\VisitsParams; +use Shlinkio\Shlink\Core\Visit\Model\WithDomainVisitsParams; use Shlinkio\Shlink\Core\Visit\VisitsStatsHelperInterface; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; +use function sprintf; + class GetTagVisitsCommand extends AbstractVisitsListCommand { public const string NAME = 'tag:visits'; + private readonly DomainOption $domainOption; + public function __construct( VisitsStatsHelperInterface $visitsHelper, private readonly ShortUrlStringifierInterface $shortUrlStringifier, ) { parent::__construct($visitsHelper); + $this->domainOption = new DomainOption($this, sprintf( + 'Return visits that belong to this domain only. Use %s keyword for visits in default domain', + Domain::DEFAULT_AUTHORITY, + )); } protected function configure(): void @@ -39,7 +49,10 @@ class GetTagVisitsCommand extends AbstractVisitsListCommand protected function getVisitsPaginator(InputInterface $input, DateRange $dateRange): Paginator { $tag = $input->getArgument('tag'); - return $this->visitsHelper->visitsForTag($tag, new VisitsParams($dateRange)); + return $this->visitsHelper->visitsForTag($tag, new WithDomainVisitsParams( + dateRange: $dateRange, + domain: $this->domainOption->get($input), + )); } /** diff --git a/module/CLI/src/Command/Tag/ListTagsCommand.php b/module/CLI/src/Command/Tag/ListTagsCommand.php index abd9a0dd..66497737 100644 --- a/module/CLI/src/Command/Tag/ListTagsCommand.php +++ b/module/CLI/src/Command/Tag/ListTagsCommand.php @@ -8,12 +8,13 @@ use Shlinkio\Shlink\CLI\Util\ShlinkTable; use Shlinkio\Shlink\Core\Tag\Model\TagInfo; use Shlinkio\Shlink\Core\Tag\Model\TagsParams; use Shlinkio\Shlink\Core\Tag\TagServiceInterface; +use Symfony\Component\Console\Attribute\AsCommand; use Symfony\Component\Console\Command\Command; -use Symfony\Component\Console\Input\InputInterface; -use Symfony\Component\Console\Output\OutputInterface; +use Symfony\Component\Console\Style\SymfonyStyle; use function array_map; +#[AsCommand(ListTagsCommand::NAME, 'Lists existing tags.')] class ListTagsCommand extends Command { public const string NAME = 'tag:list'; @@ -23,16 +24,9 @@ class ListTagsCommand extends Command parent::__construct(); } - protected function configure(): void + public function __invoke(SymfonyStyle $io): int { - $this - ->setName(self::NAME) - ->setDescription('Lists existing tags.'); - } - - protected function execute(InputInterface $input, OutputInterface $output): int - { - ShlinkTable::default($output)->render(['Name', 'URLs amount', 'Visits amount'], $this->getTagsRows()); + ShlinkTable::default($io)->render(['Name', 'URLs amount', 'Visits amount'], $this->getTagsRows()); return self::SUCCESS; } diff --git a/module/CLI/src/Command/Tag/RenameTagCommand.php b/module/CLI/src/Command/Tag/RenameTagCommand.php index 2ae0159c..f9e53f28 100644 --- a/module/CLI/src/Command/Tag/RenameTagCommand.php +++ b/module/CLI/src/Command/Tag/RenameTagCommand.php @@ -8,12 +8,12 @@ use Shlinkio\Shlink\Core\Exception\TagConflictException; use Shlinkio\Shlink\Core\Exception\TagNotFoundException; use Shlinkio\Shlink\Core\Model\Renaming; use Shlinkio\Shlink\Core\Tag\TagServiceInterface; +use Symfony\Component\Console\Attribute\Argument; +use Symfony\Component\Console\Attribute\AsCommand; use Symfony\Component\Console\Command\Command; -use Symfony\Component\Console\Input\InputArgument; -use Symfony\Component\Console\Input\InputInterface; -use Symfony\Component\Console\Output\OutputInterface; use Symfony\Component\Console\Style\SymfonyStyle; +#[AsCommand(RenameTagCommand::NAME, 'Renames one existing tag.')] class RenameTagCommand extends Command { public const string NAME = 'tag:rename'; @@ -23,21 +23,11 @@ class RenameTagCommand extends Command parent::__construct(); } - protected function configure(): void - { - $this - ->setName(self::NAME) - ->setDescription('Renames one existing tag.') - ->addArgument('oldName', InputArgument::REQUIRED, 'Current name of the tag.') - ->addArgument('newName', InputArgument::REQUIRED, 'New name of the tag.'); - } - - protected function execute(InputInterface $input, OutputInterface $output): int - { - $io = new SymfonyStyle($input, $output); - $oldName = $input->getArgument('oldName'); - $newName = $input->getArgument('newName'); - + public function __invoke( + SymfonyStyle $io, + #[Argument('Current name of the tag.')] string $oldName, + #[Argument('New name of the tag.')] string $newName, + ): int { try { $this->tagService->renameTag(Renaming::fromNames($oldName, $newName)); $io->success('Tag properly renamed.'); diff --git a/module/CLI/src/Command/Visit/DownloadGeoLiteDbCommand.php b/module/CLI/src/Command/Visit/DownloadGeoLiteDbCommand.php index 4d58a7d3..f76a4dbc 100644 --- a/module/CLI/src/Command/Visit/DownloadGeoLiteDbCommand.php +++ b/module/CLI/src/Command/Visit/DownloadGeoLiteDbCommand.php @@ -8,14 +8,17 @@ use Shlinkio\Shlink\Core\Exception\GeolocationDbUpdateFailedException; use Shlinkio\Shlink\Core\Geolocation\GeolocationDbUpdaterInterface; use Shlinkio\Shlink\Core\Geolocation\GeolocationDownloadProgressHandlerInterface; use Shlinkio\Shlink\Core\Geolocation\GeolocationResult; +use Symfony\Component\Console\Attribute\AsCommand; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Helper\ProgressBar; -use Symfony\Component\Console\Input\InputInterface; -use Symfony\Component\Console\Output\OutputInterface; use Symfony\Component\Console\Style\SymfonyStyle; use function sprintf; +#[AsCommand( + DownloadGeoLiteDbCommand::NAME, + 'Checks if the GeoLite2 db file is too old or it does not exist, and tries to download an up-to-date copy if so.', +)] class DownloadGeoLiteDbCommand extends Command implements GeolocationDownloadProgressHandlerInterface { public const string NAME = 'visit:download-db'; @@ -28,19 +31,9 @@ class DownloadGeoLiteDbCommand extends Command implements GeolocationDownloadPro parent::__construct(); } - protected function configure(): void + public function __invoke(SymfonyStyle $io): int { - $this - ->setName(self::NAME) - ->setDescription( - 'Checks if the GeoLite2 db file is too old or it does not exist, and tries to download an up-to-date ' - . 'copy if so.', - ); - } - - protected function execute(InputInterface $input, OutputInterface $output): int - { - $this->io = new SymfonyStyle($input, $output); + $this->io = $io; try { $result = $this->dbUpdater->checkDbUpdate($this); diff --git a/module/CLI/src/Command/Visit/GetNonOrphanVisitsCommand.php b/module/CLI/src/Command/Visit/GetNonOrphanVisitsCommand.php index 445bd36f..1b40d55e 100644 --- a/module/CLI/src/Command/Visit/GetNonOrphanVisitsCommand.php +++ b/module/CLI/src/Command/Visit/GetNonOrphanVisitsCommand.php @@ -4,23 +4,33 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI\Command\Visit; +use Shlinkio\Shlink\CLI\Input\DomainOption; use Shlinkio\Shlink\Common\Paginator\Paginator; use Shlinkio\Shlink\Common\Util\DateRange; +use Shlinkio\Shlink\Core\Domain\Entity\Domain; use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlStringifierInterface; use Shlinkio\Shlink\Core\Visit\Entity\Visit; -use Shlinkio\Shlink\Core\Visit\Model\VisitsParams; +use Shlinkio\Shlink\Core\Visit\Model\WithDomainVisitsParams; use Shlinkio\Shlink\Core\Visit\VisitsStatsHelperInterface; use Symfony\Component\Console\Input\InputInterface; +use function sprintf; + class GetNonOrphanVisitsCommand extends AbstractVisitsListCommand { public const string NAME = 'visit:non-orphan'; + private readonly DomainOption $domainOption; + public function __construct( VisitsStatsHelperInterface $visitsHelper, private readonly ShortUrlStringifierInterface $shortUrlStringifier, ) { parent::__construct($visitsHelper); + $this->domainOption = new DomainOption($this, sprintf( + 'Return visits that belong to this domain only. Use %s keyword for visits in default domain', + Domain::DEFAULT_AUTHORITY, + )); } protected function configure(): void @@ -35,7 +45,10 @@ class GetNonOrphanVisitsCommand extends AbstractVisitsListCommand */ protected function getVisitsPaginator(InputInterface $input, DateRange $dateRange): Paginator { - return $this->visitsHelper->nonOrphanVisits(new VisitsParams($dateRange)); + return $this->visitsHelper->nonOrphanVisits(new WithDomainVisitsParams( + dateRange: $dateRange, + domain: $this->domainOption->get($input), + )); } /** diff --git a/module/CLI/src/Command/Visit/GetOrphanVisitsCommand.php b/module/CLI/src/Command/Visit/GetOrphanVisitsCommand.php index d282d310..0804215a 100644 --- a/module/CLI/src/Command/Visit/GetOrphanVisitsCommand.php +++ b/module/CLI/src/Command/Visit/GetOrphanVisitsCommand.php @@ -4,11 +4,14 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI\Command\Visit; +use Shlinkio\Shlink\CLI\Input\DomainOption; use Shlinkio\Shlink\Common\Paginator\Paginator; use Shlinkio\Shlink\Common\Util\DateRange; +use Shlinkio\Shlink\Core\Domain\Entity\Domain; use Shlinkio\Shlink\Core\Visit\Entity\Visit; use Shlinkio\Shlink\Core\Visit\Model\OrphanVisitsParams; use Shlinkio\Shlink\Core\Visit\Model\OrphanVisitType; +use Shlinkio\Shlink\Core\Visit\VisitsStatsHelperInterface; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Input\InputOption; @@ -19,6 +22,17 @@ class GetOrphanVisitsCommand extends AbstractVisitsListCommand { public const string NAME = 'visit:orphan'; + private readonly DomainOption $domainOption; + + public function __construct(VisitsStatsHelperInterface $visitsHelper) + { + parent::__construct($visitsHelper); + $this->domainOption = new DomainOption($this, sprintf( + 'Return visits that belong to this domain only. Use %s keyword for visits in default domain', + Domain::DEFAULT_AUTHORITY, + )); + } + protected function configure(): void { $this @@ -37,7 +51,11 @@ class GetOrphanVisitsCommand extends AbstractVisitsListCommand { $rawType = $input->getOption('type'); $type = $rawType !== null ? OrphanVisitType::from($rawType) : null; - return $this->visitsHelper->orphanVisits(new OrphanVisitsParams(dateRange: $dateRange, type: $type)); + return $this->visitsHelper->orphanVisits(new OrphanVisitsParams( + dateRange: $dateRange, + domain: $this->domainOption->get($input), + type: $type, + )); } /** diff --git a/module/CLI/src/Input/DomainOption.php b/module/CLI/src/Input/DomainOption.php new file mode 100644 index 00000000..e7a15f52 --- /dev/null +++ b/module/CLI/src/Input/DomainOption.php @@ -0,0 +1,29 @@ +addOption( + name: self::NAME, + shortcut: 'd', + mode: InputOption::VALUE_REQUIRED, + description: $description, + ); + } + + public function get(InputInterface $input): string|null + { + return $input->getOption(self::NAME); + } +} diff --git a/module/CLI/src/Input/ShortUrlDataInput.php b/module/CLI/src/Input/ShortUrlDataInput.php index 1ff1de3f..908e6536 100644 --- a/module/CLI/src/Input/ShortUrlDataInput.php +++ b/module/CLI/src/Input/ShortUrlDataInput.php @@ -13,13 +13,10 @@ use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Input\InputOption; -use function array_map; -use function array_unique; -use function Shlinkio\Shlink\Core\ArrayUtils\flatten; -use function Shlinkio\Shlink\Core\splitByComma; - final readonly class ShortUrlDataInput { + private readonly TagsOption $tagsOption; + public function __construct(Command $command, private bool $longUrlAsOption = false) { if ($longUrlAsOption) { @@ -28,13 +25,9 @@ final readonly class ShortUrlDataInput $command->addArgument('longUrl', InputArgument::REQUIRED, 'The long URL to set'); } + $this->tagsOption = new TagsOption($command, 'Tags to apply to the short URL'); + $command - ->addOption( - ShortUrlDataOption::TAGS->value, - ShortUrlDataOption::TAGS->shortcut(), - InputOption::VALUE_IS_ARRAY | InputOption::VALUE_REQUIRED, - 'Tags to apply to the short URL', - ) ->addOption( ShortUrlDataOption::VALID_SINCE->value, ShortUrlDataOption::VALID_SINCE->shortcut(), @@ -117,9 +110,8 @@ final readonly class ShortUrlDataInput $maxVisits = $input->getOption('max-visits'); $data[ShortUrlInputFilter::MAX_VISITS] = $maxVisits !== null ? (int) $maxVisits : null; } - if (ShortUrlDataOption::TAGS->wasProvided($input)) { - $tags = array_unique(flatten(array_map(splitByComma(...), $input->getOption('tags')))); - $data[ShortUrlInputFilter::TAGS] = $tags; + if ($this->tagsOption->exists($input)) { + $data[ShortUrlInputFilter::TAGS] = $this->tagsOption->get($input); } if (ShortUrlDataOption::TITLE->wasProvided($input)) { $data[ShortUrlInputFilter::TITLE] = $input->getOption('title'); diff --git a/module/CLI/src/Input/ShortUrlDataOption.php b/module/CLI/src/Input/ShortUrlDataOption.php index 29c41407..4d8b582e 100644 --- a/module/CLI/src/Input/ShortUrlDataOption.php +++ b/module/CLI/src/Input/ShortUrlDataOption.php @@ -10,7 +10,6 @@ use function sprintf; enum ShortUrlDataOption: string { - case TAGS = 'tags'; case VALID_SINCE = 'valid-since'; case VALID_UNTIL = 'valid-until'; case MAX_VISITS = 'max-visits'; @@ -21,7 +20,6 @@ enum ShortUrlDataOption: string public function shortcut(): string|null { return match ($this) { - self::TAGS => 't', self::VALID_SINCE => 's', self::VALID_UNTIL => 'u', self::MAX_VISITS => 'm', diff --git a/module/CLI/src/Input/TagsOption.php b/module/CLI/src/Input/TagsOption.php new file mode 100644 index 00000000..ff02a735 --- /dev/null +++ b/module/CLI/src/Input/TagsOption.php @@ -0,0 +1,51 @@ +addOption( + 'tag', + 't', + InputOption::VALUE_REQUIRED | InputOption::VALUE_IS_ARRAY, + $description, + ) + ->addOption( + 'tags', + mode: InputOption::VALUE_REQUIRED | InputOption::VALUE_IS_ARRAY, + description: '[DEPRECATED] Use --tag instead', + ); + } + + /** + * Whether tags have been set or not, via `--tag`, `-t` or the deprecated `--tags` + */ + public function exists(InputInterface $input): bool + { + return $input->hasParameterOption(['--tag', '-t']) || $input->hasParameterOption('--tags'); + } + + /** + * @return string[] + */ + public function get(InputInterface $input): array + { + // FIXME DEPRECATED Remove support for comma-separated tags in next major release + $tags = [...$input->getOption('tag'), ...$input->getOption('tags')]; + return array_unique(flatten(array_map(splitByComma(...), $tags))); + } +} diff --git a/module/CLI/test-cli/Command/ListShortUrlsTest.php b/module/CLI/test-cli/Command/ListShortUrlsTest.php index d7c50912..c01a631f 100644 --- a/module/CLI/test-cli/Command/ListShortUrlsTest.php +++ b/module/CLI/test-cli/Command/ListShortUrlsTest.php @@ -87,6 +87,15 @@ class ListShortUrlsTest extends CliTestCase | ghi789 | | http://s.test/ghi789 | https://shlink.io/documentation/ | 2018-05-01T00:00:00+00:00 | 2 | +------------+---------------+----------------------+--------------------------------------- Page 1 of 1 -------------------------------------------------+---------------------------+--------------+ OUTPUT]; + yield 'exclude tags' => [['--exclude-tag=foo'], <<apiKeyService = $this->createMock(ApiKeyServiceInterface::class); + $this->commandTester = CliTestUtils::testerForCommand(new DeleteKeyCommand($this->apiKeyService)); + } + + #[Test] + public function warningIsReturnedIfNoArgumentIsProvidedInNonInteractiveMode(): void + { + $this->apiKeyService->expects($this->never())->method('deleteByName'); + $this->apiKeyService->expects($this->never())->method('listKeys'); + + $exitCode = $this->commandTester->execute([], ['interactive' => false]); + + self::assertEquals(Command::INVALID, $exitCode); + } + + #[Test] + public function confirmationIsSkippedInNonInteractiveMode(): void + { + $this->apiKeyService->expects($this->once())->method('deleteByName'); + $this->apiKeyService->expects($this->never())->method('listKeys'); + + $exitCode = $this->commandTester->execute(['name' => 'key to delete'], ['interactive' => false]); + $output = $this->commandTester->getDisplay(); + + self::assertEquals(Command::SUCCESS, $exitCode); + self::assertStringNotContainsString('Are you sure you want to delete the API key?', $output); + } + + #[Test] + public function keyIsNotDeletedIfConfirmationIsCancelled(): void + { + $this->apiKeyService->expects($this->never())->method('deleteByName'); + $this->apiKeyService->expects($this->never())->method('listKeys'); + + $this->commandTester->setInputs(['no']); + $exitCode = $this->commandTester->execute(['name' => 'key_to_delete']); + + self::assertEquals(Command::INVALID, $exitCode); + } + + #[Test] + public function existingApiKeyNamesAreListedIfNoArgumentIsProvidedInInteractiveMode(): void + { + $name = 'the key to delete'; + $this->apiKeyService->expects($this->once())->method('deleteByName')->with($name); + $this->apiKeyService->expects($this->once())->method('listKeys')->willReturn([ + ApiKey::fromMeta(ApiKeyMeta::fromParams(name: 'foo')), + ApiKey::fromMeta(ApiKeyMeta::fromParams(name: $name)), + ApiKey::fromMeta(ApiKeyMeta::fromParams(name: 'bar')), + ]); + + $this->commandTester->setInputs([$name, 'y']); + $exitCode = $this->commandTester->execute([]); + $output = $this->commandTester->getDisplay(); + + self::assertStringContainsString('What API key do you want to delete?', $output); + self::assertStringContainsString('API key "the key to delete" properly deleted', $output); + self::assertEquals(Command::SUCCESS, $exitCode); + } + + #[Test] + public function errorIsReturnedIfDisableByKeyThrowsException(): void + { + $apiKey = 'key to delete'; + $e = ApiKeyNotFoundException::forName($apiKey); + $this->apiKeyService->expects($this->once())->method('deleteByName')->with($apiKey)->willThrowException($e); + $this->apiKeyService->expects($this->never())->method('listKeys'); + + $exitCode = $this->commandTester->execute(['name' => $apiKey]); + $output = $this->commandTester->getDisplay(); + + self::assertStringContainsString($e->getMessage(), $output); + self::assertEquals(Command::FAILURE, $exitCode); + } +} diff --git a/module/CLI/test/Command/Api/InitialApiKeyCommandTest.php b/module/CLI/test/Command/Api/InitialApiKeyCommandTest.php index e86cf0e5..b2311613 100644 --- a/module/CLI/test/Command/Api/InitialApiKeyCommandTest.php +++ b/module/CLI/test/Command/Api/InitialApiKeyCommandTest.php @@ -35,7 +35,7 @@ class InitialApiKeyCommandTest extends TestCase $this->apiKeyService->expects($this->once())->method('createInitial')->with('the_key')->willReturn($result); $this->commandTester->execute( - ['apiKey' => 'the_key'], + ['api-key' => 'the_key'], ['verbosity' => $verbose ? OutputInterface::VERBOSITY_VERBOSE : OutputInterface::VERBOSITY_NORMAL], ); $output = $this->commandTester->getDisplay(); diff --git a/module/CLI/test/Command/Config/ReadEnvVarCommandTest.php b/module/CLI/test/Command/Config/ReadEnvVarCommandTest.php index c377cf86..e90f94af 100644 --- a/module/CLI/test/Command/Config/ReadEnvVarCommandTest.php +++ b/module/CLI/test/Command/Config/ReadEnvVarCommandTest.php @@ -28,13 +28,13 @@ class ReadEnvVarCommandTest extends TestCase $this->expectException(InvalidArgumentException::class); $this->expectExceptionMessage('foo is not a valid Shlink environment variable'); - $this->commandTester->execute(['envVar' => 'foo']); + $this->commandTester->execute(['env-var' => 'foo']); } #[Test] public function valueIsPrintedIfProvidedEnvVarIsValid(): void { - $this->commandTester->execute(['envVar' => EnvVars::BASE_PATH->value]); + $this->commandTester->execute(['env-var' => EnvVars::BASE_PATH->value]); $output = $this->commandTester->getDisplay(); self::assertStringNotContainsString('Select the env var to read', $output); diff --git a/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php b/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php index 0a7f9aa0..8d75322e 100644 --- a/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php +++ b/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php @@ -25,8 +25,6 @@ use Shlinkio\Shlink\Rest\Entity\ApiKey; use ShlinkioTest\Shlink\CLI\Util\CliTestUtils; use Symfony\Component\Console\Tester\CommandTester; -use function explode; - class ListShortUrlsCommandTest extends TestCase { private CommandTester $commandTester; @@ -209,6 +207,9 @@ class ListShortUrlsCommandTest extends TestCase string $tagsMode, string|null $startDate = null, string|null $endDate = null, + array $excludeTags = [], + string $excludeTagsMode = TagsMode::ANY->value, + string|null $apiKeyName = null, ): void { $this->shortUrlService->expects($this->once())->method('listShortUrls')->with(ShortUrlsParams::fromRawData([ 'page' => $page, @@ -217,6 +218,9 @@ class ListShortUrlsCommandTest extends TestCase 'tagsMode' => $tagsMode, 'startDate' => $startDate !== null ? Chronos::parse($startDate)->toAtomString() : null, 'endDate' => $endDate !== null ? Chronos::parse($endDate)->toAtomString() : null, + 'excludeTags' => $excludeTags, + 'excludeTagsMode' => $excludeTagsMode, + 'apiKeyName' => $apiKeyName, ]))->willReturn(new Paginator(new ArrayAdapter([]))); $this->commandTester->setInputs(['n']); @@ -230,10 +234,10 @@ class ListShortUrlsCommandTest extends TestCase yield [['--including-all-tags' => true], 1, null, [], TagsMode::ALL->value]; yield [['--search-term' => $searchTerm = 'search this'], 1, $searchTerm, [], TagsMode::ANY->value]; yield [ - ['--page' => $page = 3, '--search-term' => $searchTerm = 'search this', '--tags' => $tags = 'foo,bar'], + ['--page' => $page = 3, '--search-term' => $searchTerm = 'search this', '--tag' => $tags = ['foo', 'bar']], $page, $searchTerm, - explode(',', $tags), + $tags, TagsMode::ANY->value, ]; yield [ @@ -262,6 +266,29 @@ class ListShortUrlsCommandTest extends TestCase $startDate, $endDate, ]; + yield [ + ['--exclude-tag' => ['foo', 'bar'], '--exclude-tags-all' => true], + 1, + null, + [], + TagsMode::ANY->value, + null, + null, + ['foo', 'bar'], + TagsMode::ALL->value, + ]; + yield [ + ['--api-key-name' => 'foo'], + 1, + null, + [], + TagsMode::ANY->value, + null, + null, + [], + TagsMode::ANY->value, + 'foo', + ]; } #[Test, DataProvider('provideOrderBy')] diff --git a/module/CLI/test/Command/Tag/RenameTagCommandTest.php b/module/CLI/test/Command/Tag/RenameTagCommandTest.php index e7fb630d..8681239a 100644 --- a/module/CLI/test/Command/Tag/RenameTagCommandTest.php +++ b/module/CLI/test/Command/Tag/RenameTagCommandTest.php @@ -36,8 +36,8 @@ class RenameTagCommandTest extends TestCase )->willThrowException(TagNotFoundException::fromTag('foo')); $this->commandTester->execute([ - 'oldName' => $oldName, - 'newName' => $newName, + 'old-name' => $oldName, + 'new-name' => $newName, ]); $output = $this->commandTester->getDisplay(); @@ -54,8 +54,8 @@ class RenameTagCommandTest extends TestCase )->willReturn(new Tag($newName)); $this->commandTester->execute([ - 'oldName' => $oldName, - 'newName' => $newName, + 'old-name' => $oldName, + 'new-name' => $newName, ]); $output = $this->commandTester->getDisplay(); diff --git a/module/CLI/test/Util/ShlinkTableTest.php b/module/CLI/test/Util/ShlinkTableTest.php index 19d477ef..e80e1f8b 100644 --- a/module/CLI/test/Util/ShlinkTableTest.php +++ b/module/CLI/test/Util/ShlinkTableTest.php @@ -51,7 +51,6 @@ class ShlinkTableTest extends TestCase $ref = new ReflectionObject($instance); $baseTable = $ref->getProperty('baseTable'); - $baseTable->setAccessible(true); self::assertInstanceOf(Table::class, $baseTable->getValue($instance)); } diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index 0ad943e9..5bb534c2 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -168,7 +168,7 @@ return [ ], Visit\Geolocation\VisitLocator::class => ['em', Visit\Repository\VisitIterationRepository::class], Visit\Geolocation\VisitToLocationHelper::class => [IpLocationResolverInterface::class], - Visit\VisitsStatsHelper::class => ['em'], + Visit\VisitsStatsHelper::class => ['em', Config\Options\UrlShortenerOptions::class], Tag\TagService::class => ['em', Tag\Repository\TagRepository::class], ShortUrl\DeleteShortUrlService::class => [ 'em', diff --git a/module/Core/src/Config/EnvVars.php b/module/Core/src/Config/EnvVars.php index bf5acc3e..8613b53f 100644 --- a/module/Core/src/Config/EnvVars.php +++ b/module/Core/src/Config/EnvVars.php @@ -41,6 +41,8 @@ enum EnvVars: string case CACHE_NAMESPACE = 'CACHE_NAMESPACE'; case REDIS_SERVERS = 'REDIS_SERVERS'; case REDIS_SENTINEL_SERVICE = 'REDIS_SENTINEL_SERVICE'; + case REDIS_SERVERS_USER = 'REDIS_SERVERS_USER'; + case REDIS_SERVERS_PASSWORD = 'REDIS_SERVERS_PASSWORD'; case REDIS_PUB_SUB_ENABLED = 'REDIS_PUB_SUB_ENABLED'; case MERCURE_ENABLED = 'MERCURE_ENABLED'; case MERCURE_PUBLIC_HUB_URL = 'MERCURE_PUBLIC_HUB_URL'; diff --git a/module/Core/src/Model/AbstractInfinitePaginableListParams.php b/module/Core/src/Model/AbstractInfinitePaginableListParams.php index 70db853e..4d1f5f89 100644 --- a/module/Core/src/Model/AbstractInfinitePaginableListParams.php +++ b/module/Core/src/Model/AbstractInfinitePaginableListParams.php @@ -8,7 +8,7 @@ use Shlinkio\Shlink\Common\Paginator\Paginator; abstract class AbstractInfinitePaginableListParams { - private const FIRST_PAGE = 1; + private const int FIRST_PAGE = 1; public readonly int $page; public readonly int $itemsPerPage; diff --git a/module/Core/src/ShortUrl/Model/ShortUrlsParams.php b/module/Core/src/ShortUrl/Model/ShortUrlsParams.php index 7b68ed37..b8b34904 100644 --- a/module/Core/src/ShortUrl/Model/ShortUrlsParams.php +++ b/module/Core/src/ShortUrl/Model/ShortUrlsParams.php @@ -12,21 +12,27 @@ use Shlinkio\Shlink\Core\ShortUrl\Model\Validation\ShortUrlsParamsInputFilter; use function Shlinkio\Shlink\Common\buildDateRange; use function Shlinkio\Shlink\Core\normalizeOptionalDate; -final class ShortUrlsParams +/** + * Represents all the params that can be used to filter a list of short URLs + */ +final readonly class ShortUrlsParams { - public const DEFAULT_ITEMS_PER_PAGE = 10; + public const int DEFAULT_ITEMS_PER_PAGE = 10; private function __construct( - public readonly int $page, - public readonly int $itemsPerPage, - public readonly string|null $searchTerm, - public readonly array $tags, - public readonly Ordering $orderBy, - public readonly DateRange|null $dateRange, - public readonly bool $excludeMaxVisitsReached, - public readonly bool $excludePastValidUntil, - public readonly TagsMode $tagsMode = TagsMode::ANY, - public readonly string|null $domain = null, + public int $page, + public int $itemsPerPage, + public string|null $searchTerm, + public array $tags, + public Ordering $orderBy, + public DateRange|null $dateRange, + public bool $excludeMaxVisitsReached, + public bool $excludePastValidUntil, + public TagsMode $tagsMode = TagsMode::ANY, + public string|null $domain = null, + public array $excludeTags = [], + public TagsMode $excludeTagsMode = TagsMode::ANY, + public string|null $apiKeyName = null, ) { } @@ -61,6 +67,11 @@ final class ShortUrlsParams excludePastValidUntil: $inputFilter->getValue(ShortUrlsParamsInputFilter::EXCLUDE_PAST_VALID_UNTIL), tagsMode: self::resolveTagsMode($inputFilter->getValue(ShortUrlsParamsInputFilter::TAGS_MODE)), domain: $inputFilter->getValue(ShortUrlsParamsInputFilter::DOMAIN), + excludeTags: (array) $inputFilter->getValue(ShortUrlsParamsInputFilter::EXCLUDE_TAGS), + excludeTagsMode: self::resolveTagsMode( + $inputFilter->getValue(ShortUrlsParamsInputFilter::EXCLUDE_TAGS_MODE), + ), + apiKeyName: $inputFilter->getValue(ShortUrlsParamsInputFilter::API_KEY_NAME), ); } diff --git a/module/Core/src/ShortUrl/Model/Validation/ShortUrlsParamsInputFilter.php b/module/Core/src/ShortUrl/Model/Validation/ShortUrlsParamsInputFilter.php index bc9de337..73c54e3a 100644 --- a/module/Core/src/ShortUrl/Model/Validation/ShortUrlsParamsInputFilter.php +++ b/module/Core/src/ShortUrl/Model/Validation/ShortUrlsParamsInputFilter.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\ShortUrl\Model\Validation; +use Laminas\InputFilter\Input; use Laminas\InputFilter\InputFilter; use Laminas\Validator\InArray; use Shlinkio\Shlink\Common\Paginator\Paginator; @@ -19,14 +20,17 @@ class ShortUrlsParamsInputFilter extends InputFilter public const string PAGE = 'page'; public const string SEARCH_TERM = 'searchTerm'; public const string TAGS = 'tags'; + public const string TAGS_MODE = 'tagsMode'; + public const string EXCLUDE_TAGS = 'excludeTags'; + public const string EXCLUDE_TAGS_MODE = 'excludeTagsMode'; public const string START_DATE = 'startDate'; public const string END_DATE = 'endDate'; public const string ITEMS_PER_PAGE = 'itemsPerPage'; - public const string TAGS_MODE = 'tagsMode'; public const string ORDER_BY = 'orderBy'; public const string EXCLUDE_MAX_VISITS_REACHED = 'excludeMaxVisitsReached'; public const string EXCLUDE_PAST_VALID_UNTIL = 'excludePastValidUntil'; public const string DOMAIN = 'domain'; + public const string API_KEY_NAME = 'apiKeyName'; public function __construct(array $data) { @@ -45,13 +49,10 @@ class ShortUrlsParamsInputFilter extends InputFilter $this->add(InputFactory::numeric(self::ITEMS_PER_PAGE, Paginator::ALL_ITEMS)); $this->add(InputFactory::tags(self::TAGS)); + $this->add($this->createTagsModeInput(self::TAGS_MODE)); - $tagsMode = InputFactory::basic(self::TAGS_MODE); - $tagsMode->getValidatorChain()->attach(new InArray([ - 'haystack' => enumValues(TagsMode::class), - 'strict' => InArray::COMPARE_STRICT, - ])); - $this->add($tagsMode); + $this->add(InputFactory::tags(self::EXCLUDE_TAGS)); + $this->add($this->createTagsModeInput(self::EXCLUDE_TAGS_MODE)); $this->add(InputFactory::orderBy(self::ORDER_BY, enumValues(OrderableField::class))); @@ -59,5 +60,17 @@ class ShortUrlsParamsInputFilter extends InputFilter $this->add(InputFactory::boolean(self::EXCLUDE_PAST_VALID_UNTIL)); $this->add(InputFactory::basic(self::DOMAIN)); + $this->add(InputFactory::basic(self::API_KEY_NAME)); + } + + private function createTagsModeInput(string $name): Input + { + $tagsMode = InputFactory::basic($name); + $tagsMode->getValidatorChain()->attach(new InArray([ + 'haystack' => enumValues(TagsMode::class), + 'strict' => InArray::COMPARE_STRICT, + ])); + + return $tagsMode; } } diff --git a/module/Core/src/ShortUrl/Persistence/ShortUrlsCountFiltering.php b/module/Core/src/ShortUrl/Persistence/ShortUrlsCountFiltering.php index a8e42236..8fa44408 100644 --- a/module/Core/src/ShortUrl/Persistence/ShortUrlsCountFiltering.php +++ b/module/Core/src/ShortUrl/Persistence/ShortUrlsCountFiltering.php @@ -15,22 +15,33 @@ use function strtolower; class ShortUrlsCountFiltering { public readonly bool $searchIncludesDefaultDomain; + public readonly string|null $apiKeyName; + /** + * @param $defaultDomain - Used only to determine if search term includes default domain + */ public function __construct( public readonly string|null $searchTerm = null, public readonly array $tags = [], - public readonly TagsMode|null $tagsMode = null, + public readonly TagsMode $tagsMode = TagsMode::ANY, public readonly DateRange|null $dateRange = null, public readonly bool $excludeMaxVisitsReached = false, public readonly bool $excludePastValidUntil = false, public readonly ApiKey|null $apiKey = null, string|null $defaultDomain = null, public readonly string|null $domain = null, + public readonly array $excludeTags = [], + public readonly TagsMode $excludeTagsMode = TagsMode::ANY, + string|null $apiKeyName = null, ) { $this->searchIncludesDefaultDomain = !empty($searchTerm) && !empty($defaultDomain) && str_contains( strtolower($defaultDomain), strtolower($searchTerm), ); + + // Filtering by API key name is only allowed if the API key used in the request is an admin one, or it matches + // the API key name + $this->apiKeyName = $apiKey?->name === $apiKeyName || ApiKey::isAdmin($apiKey) ? $apiKeyName : null; } public static function fromParams(ShortUrlsParams $params, ApiKey|null $apiKey, string $defaultDomain): self @@ -45,6 +56,9 @@ class ShortUrlsCountFiltering $apiKey, $defaultDomain, $params->domain, + $params->excludeTags, + $params->excludeTagsMode, + $params->apiKeyName, ); } } diff --git a/module/Core/src/ShortUrl/Persistence/ShortUrlsListFiltering.php b/module/Core/src/ShortUrl/Persistence/ShortUrlsListFiltering.php index d0fa6418..f9350389 100644 --- a/module/Core/src/ShortUrl/Persistence/ShortUrlsListFiltering.php +++ b/module/Core/src/ShortUrl/Persistence/ShortUrlsListFiltering.php @@ -12,20 +12,25 @@ use Shlinkio\Shlink\Rest\Entity\ApiKey; class ShortUrlsListFiltering extends ShortUrlsCountFiltering { + /** + * @inheritDoc + */ public function __construct( public readonly int|null $limit = null, public readonly int|null $offset = null, public readonly Ordering $orderBy = new Ordering(), string|null $searchTerm = null, array $tags = [], - TagsMode|null $tagsMode = null, + TagsMode $tagsMode = TagsMode::ANY, DateRange|null $dateRange = null, bool $excludeMaxVisitsReached = false, bool $excludePastValidUntil = false, ApiKey|null $apiKey = null, - // Used only to determine if search term includes default domain string|null $defaultDomain = null, string|null $domain = null, + array $excludeTags = [], + TagsMode $excludeTagsMode = TagsMode::ANY, + string|null $apiKeyName = null, ) { parent::__construct( $searchTerm, @@ -37,6 +42,9 @@ class ShortUrlsListFiltering extends ShortUrlsCountFiltering $apiKey, $defaultDomain, $domain, + $excludeTags, + $excludeTagsMode, + $apiKeyName, ); } @@ -60,6 +68,9 @@ class ShortUrlsListFiltering extends ShortUrlsCountFiltering $apiKey, $defaultDomain, $params->domain, + $params->excludeTags, + $params->excludeTagsMode, + $params->apiKeyName, ); } } diff --git a/module/Core/src/ShortUrl/Repository/ShortUrlListRepository.php b/module/Core/src/ShortUrl/Repository/ShortUrlListRepository.php index c18b31ef..8b720471 100644 --- a/module/Core/src/ShortUrl/Repository/ShortUrlListRepository.php +++ b/module/Core/src/ShortUrl/Repository/ShortUrlListRepository.php @@ -105,6 +105,10 @@ class ShortUrlListRepository extends EntitySpecificationRepository implements Sh $searchTerm = $filtering->searchTerm; $tags = $filtering->tags; + $tagsMode = $filtering->tagsMode; + $excludeTags = $filtering->excludeTags; + $excludeTagsMode = $filtering->excludeTagsMode; + if (! empty($searchTerm)) { // Left join with tags only if no tags were provided. In case of tags, an inner join will be done later if (empty($tags)) { @@ -125,7 +129,6 @@ class ShortUrlListRepository extends EntitySpecificationRepository implements Sh } // Apply tag conditions, only when not filtering by all provided tags - $tagsMode = $filtering->tagsMode ?? TagsMode::ANY; if (empty($tags) || $tagsMode === TagsMode::ANY) { $conditions[] = $qb->expr()->like('t.name', ':searchPattern'); } @@ -134,21 +137,33 @@ class ShortUrlListRepository extends EntitySpecificationRepository implements Sh ->setParameter('searchPattern', '%' . $searchTerm . '%'); } - // Filter by tags if provided if (! empty($tags)) { - $tagsMode = $filtering->tagsMode ?? TagsMode::ANY; - $tagsMode === TagsMode::ANY - ? $qb->join('s.tags', 't')->andWhere($qb->expr()->in('t.name', $tags)) - : $this->joinAllTags($qb, $tags); + if ($tagsMode === TagsMode::ANY) { + $qb->join('s.tags', 't')->andWhere($qb->expr()->in('t.name', $tags)); + } else { + $this->joinAllTags($qb, $tags); + } } - if ($filtering->domain !== null) { - if ($filtering->domain === Domain::DEFAULT_AUTHORITY) { - $qb->andWhere($qb->expr()->isNull('s.domain')); + if (! empty($excludeTags)) { + $subQb = $this->getEntityManager()->createQueryBuilder(); + $subQb->select('s2.id') + ->from(ShortUrl::class, 's2'); + + if ($excludeTagsMode === TagsMode::ANY) { + $subQb->join('s2.tags', 't2')->andWhere($qb->expr()->in('t2.name', $excludeTags)); } else { - $qb->andWhere($qb->expr()->eq('d.authority', ':domain')) - ->setParameter('domain', $filtering->domain); + $this->joinAllTags($subQb, $excludeTags, shortUrlsAlias: 's2', boundParamsQb: $qb); } + + $qb->andWhere($qb->expr()->notIn('s.id', $subQb->getDQL())); + } + + if ($filtering->domain === Domain::DEFAULT_AUTHORITY) { + $qb->andWhere($qb->expr()->isNull('s.domain')); + } elseif ($filtering->domain !== null) { + $qb->andWhere($qb->expr()->eq('d.authority', ':domain')) + ->setParameter('domain', $filtering->domain); } if ($filtering->excludeMaxVisitsReached) { @@ -173,17 +188,40 @@ class ShortUrlListRepository extends EntitySpecificationRepository implements Sh ->setParameter('minValidUntil', Chronos::now()->toDateTimeString()); } + $apiKeyName = $filtering->apiKeyName; + if ($apiKeyName !== null) { + $qb + ->join('s.authorApiKey', 'a') + ->andWhere($qb->expr()->eq('a.name', ':apiKeyName')) + ->setParameter('apiKeyName', $apiKeyName); + } + $this->applySpecification($qb, $filtering->apiKey?->spec(), 's'); return $qb; } - private function joinAllTags(QueryBuilder $qb, array $tags): void - { + /** + * @param $boundParamsQb - The query builder in which params should be bound, in case the main provided QB is going + * to be used as a sub query, since params need to be bound in the parent query. + * Defaults to the main $qb + */ + private function joinAllTags( + QueryBuilder $qb, + array $tags, + string $shortUrlsAlias = 's', + QueryBuilder|null $boundParamsQb = null, + ): void { + $boundParamsQb ??= $qb; foreach ($tags as $index => $tag) { - $alias = 't_' . $index; - $qb->join('s.tags', $alias, Join::WITH, $alias . '.name = :tag' . $index) - ->setParameter('tag' . $index, $tag); + $alias = 't_' . $index . $shortUrlsAlias; + $qb->join( + $shortUrlsAlias . '.tags', + $alias, + Join::WITH, + $alias . '.name = :tag' . $index . $shortUrlsAlias, + ); + $boundParamsQb->setParameter('tag' . $index . $shortUrlsAlias, $tag); } } } diff --git a/module/Core/src/Visit/Model/OrphanVisitsParams.php b/module/Core/src/Visit/Model/OrphanVisitsParams.php index 6991928d..bff5b38c 100644 --- a/module/Core/src/Visit/Model/OrphanVisitsParams.php +++ b/module/Core/src/Visit/Model/OrphanVisitsParams.php @@ -9,20 +9,22 @@ use ValueError; use function Shlinkio\Shlink\Core\enumToString; use function sprintf; -final class OrphanVisitsParams extends VisitsParams +final class OrphanVisitsParams extends WithDomainVisitsParams { public function __construct( DateRange|null $dateRange = null, int|null $page = null, int|null $itemsPerPage = null, bool $excludeBots = false, + string|null $domain = null, public readonly OrphanVisitType|null $type = null, ) { - parent::__construct($dateRange, $page, $itemsPerPage, $excludeBots); + parent::__construct($dateRange, $page, $itemsPerPage, $excludeBots, $domain); } - public static function fromVisitsParamsAndRawData(VisitsParams $visitsParams, array $query): self + public static function fromRawData(array $query): self { + $visitsParams = WithDomainVisitsParams::fromRawData($query); $type = $query['type'] ?? null; return new self( @@ -30,6 +32,7 @@ final class OrphanVisitsParams extends VisitsParams page: $visitsParams->page, itemsPerPage: $visitsParams->itemsPerPage, excludeBots: $visitsParams->excludeBots, + domain: $visitsParams->domain, type: $type !== null ? self::parseType($type) : null, ); } diff --git a/module/Core/src/Visit/Model/WithDomainVisitsParams.php b/module/Core/src/Visit/Model/WithDomainVisitsParams.php new file mode 100644 index 00000000..edec3399 --- /dev/null +++ b/module/Core/src/Visit/Model/WithDomainVisitsParams.php @@ -0,0 +1,31 @@ +dateRange, + page: $visitsParams->page, + itemsPerPage: $visitsParams->itemsPerPage, + excludeBots: $visitsParams->excludeBots, + domain: $query['domain'] ?? null, + ); + } +} diff --git a/module/Core/src/Visit/Paginator/Adapter/NonOrphanVisitsPaginatorAdapter.php b/module/Core/src/Visit/Paginator/Adapter/NonOrphanVisitsPaginatorAdapter.php index 5e3cdbe1..e23cf10c 100644 --- a/module/Core/src/Visit/Paginator/Adapter/NonOrphanVisitsPaginatorAdapter.php +++ b/module/Core/src/Visit/Paginator/Adapter/NonOrphanVisitsPaginatorAdapter.php @@ -6,9 +6,9 @@ namespace Shlinkio\Shlink\Core\Visit\Paginator\Adapter; use Shlinkio\Shlink\Core\Paginator\Adapter\AbstractCacheableCountPaginatorAdapter; use Shlinkio\Shlink\Core\Visit\Entity\Visit; -use Shlinkio\Shlink\Core\Visit\Model\VisitsParams; -use Shlinkio\Shlink\Core\Visit\Persistence\VisitsCountFiltering; -use Shlinkio\Shlink\Core\Visit\Persistence\VisitsListFiltering; +use Shlinkio\Shlink\Core\Visit\Model\WithDomainVisitsParams; +use Shlinkio\Shlink\Core\Visit\Persistence\WithDomainVisitsCountFiltering; +use Shlinkio\Shlink\Core\Visit\Persistence\WithDomainVisitsListFiltering; use Shlinkio\Shlink\Core\Visit\Repository\VisitRepositoryInterface; use Shlinkio\Shlink\Rest\Entity\ApiKey; @@ -17,26 +17,28 @@ class NonOrphanVisitsPaginatorAdapter extends AbstractCacheableCountPaginatorAda { public function __construct( private readonly VisitRepositoryInterface $repo, - private readonly VisitsParams $params, + private readonly WithDomainVisitsParams $params, private readonly ApiKey|null $apiKey, ) { } protected function doCount(): int { - return $this->repo->countNonOrphanVisits(new VisitsCountFiltering( + return $this->repo->countNonOrphanVisits(new WithDomainVisitsCountFiltering( $this->params->dateRange, $this->params->excludeBots, $this->apiKey, + $this->params->domain, )); } public function getSlice(int $offset, int $length): iterable { - return $this->repo->findNonOrphanVisits(new VisitsListFiltering( + return $this->repo->findNonOrphanVisits(new WithDomainVisitsListFiltering( $this->params->dateRange, $this->params->excludeBots, $this->apiKey, + $this->params->domain, $length, $offset, )); diff --git a/module/Core/src/Visit/Paginator/Adapter/OrphanVisitsPaginatorAdapter.php b/module/Core/src/Visit/Paginator/Adapter/OrphanVisitsPaginatorAdapter.php index 899ab831..ad175a79 100644 --- a/module/Core/src/Visit/Paginator/Adapter/OrphanVisitsPaginatorAdapter.php +++ b/module/Core/src/Visit/Paginator/Adapter/OrphanVisitsPaginatorAdapter.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Visit\Paginator\Adapter; +use Shlinkio\Shlink\Core\Config\Options\UrlShortenerOptions; use Shlinkio\Shlink\Core\Paginator\Adapter\AbstractCacheableCountPaginatorAdapter; use Shlinkio\Shlink\Core\Visit\Entity\Visit; use Shlinkio\Shlink\Core\Visit\Model\OrphanVisitsParams; @@ -19,6 +20,7 @@ class OrphanVisitsPaginatorAdapter extends AbstractCacheableCountPaginatorAdapte private readonly VisitRepositoryInterface $repo, private readonly OrphanVisitsParams $params, private readonly ApiKey|null $apiKey, + private readonly UrlShortenerOptions $options, ) { } @@ -28,7 +30,9 @@ class OrphanVisitsPaginatorAdapter extends AbstractCacheableCountPaginatorAdapte dateRange: $this->params->dateRange, excludeBots: $this->params->excludeBots, apiKey: $this->apiKey, + domain: $this->params->domain, type: $this->params->type, + defaultDomain: $this->options->defaultDomain, )); } @@ -38,7 +42,9 @@ class OrphanVisitsPaginatorAdapter extends AbstractCacheableCountPaginatorAdapte dateRange: $this->params->dateRange, excludeBots: $this->params->excludeBots, apiKey: $this->apiKey, + domain: $this->params->domain, type: $this->params->type, + defaultDomain: $this->options->defaultDomain, limit: $length, offset: $offset, )); diff --git a/module/Core/src/Visit/Paginator/Adapter/TagVisitsPaginatorAdapter.php b/module/Core/src/Visit/Paginator/Adapter/TagVisitsPaginatorAdapter.php index 909bd2ba..a76b7524 100644 --- a/module/Core/src/Visit/Paginator/Adapter/TagVisitsPaginatorAdapter.php +++ b/module/Core/src/Visit/Paginator/Adapter/TagVisitsPaginatorAdapter.php @@ -6,9 +6,9 @@ namespace Shlinkio\Shlink\Core\Visit\Paginator\Adapter; use Shlinkio\Shlink\Core\Paginator\Adapter\AbstractCacheableCountPaginatorAdapter; use Shlinkio\Shlink\Core\Visit\Entity\Visit; -use Shlinkio\Shlink\Core\Visit\Model\VisitsParams; -use Shlinkio\Shlink\Core\Visit\Persistence\VisitsCountFiltering; -use Shlinkio\Shlink\Core\Visit\Persistence\VisitsListFiltering; +use Shlinkio\Shlink\Core\Visit\Model\WithDomainVisitsParams; +use Shlinkio\Shlink\Core\Visit\Persistence\WithDomainVisitsCountFiltering; +use Shlinkio\Shlink\Core\Visit\Persistence\WithDomainVisitsListFiltering; use Shlinkio\Shlink\Core\Visit\Repository\VisitRepositoryInterface; use Shlinkio\Shlink\Rest\Entity\ApiKey; @@ -18,7 +18,7 @@ class TagVisitsPaginatorAdapter extends AbstractCacheableCountPaginatorAdapter public function __construct( private readonly VisitRepositoryInterface $visitRepository, private readonly string $tag, - private readonly VisitsParams $params, + private readonly WithDomainVisitsParams $params, private readonly ApiKey|null $apiKey, ) { } @@ -27,10 +27,11 @@ class TagVisitsPaginatorAdapter extends AbstractCacheableCountPaginatorAdapter { return $this->visitRepository->findVisitsByTag( $this->tag, - new VisitsListFiltering( + new WithDomainVisitsListFiltering( $this->params->dateRange, $this->params->excludeBots, $this->apiKey, + $this->params->domain, $length, $offset, ), @@ -41,10 +42,11 @@ class TagVisitsPaginatorAdapter extends AbstractCacheableCountPaginatorAdapter { return $this->visitRepository->countVisitsByTag( $this->tag, - new VisitsCountFiltering( + new WithDomainVisitsCountFiltering( $this->params->dateRange, $this->params->excludeBots, $this->apiKey, + $this->params->domain, ), ); } diff --git a/module/Core/src/Visit/Persistence/OrphanVisitsCountFiltering.php b/module/Core/src/Visit/Persistence/OrphanVisitsCountFiltering.php index c09bc5ca..84f7af57 100644 --- a/module/Core/src/Visit/Persistence/OrphanVisitsCountFiltering.php +++ b/module/Core/src/Visit/Persistence/OrphanVisitsCountFiltering.php @@ -8,14 +8,16 @@ use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\Visit\Model\OrphanVisitType; use Shlinkio\Shlink\Rest\Entity\ApiKey; -class OrphanVisitsCountFiltering extends VisitsCountFiltering +class OrphanVisitsCountFiltering extends WithDomainVisitsCountFiltering { public function __construct( DateRange|null $dateRange = null, bool $excludeBots = false, ApiKey|null $apiKey = null, + string|null $domain = null, public readonly OrphanVisitType|null $type = null, + public readonly string $defaultDomain = '', ) { - parent::__construct($dateRange, $excludeBots, $apiKey); + parent::__construct($dateRange, $excludeBots, $apiKey, $domain); } } diff --git a/module/Core/src/Visit/Persistence/OrphanVisitsListFiltering.php b/module/Core/src/Visit/Persistence/OrphanVisitsListFiltering.php index d1e49605..23c3d6cb 100644 --- a/module/Core/src/Visit/Persistence/OrphanVisitsListFiltering.php +++ b/module/Core/src/Visit/Persistence/OrphanVisitsListFiltering.php @@ -14,10 +14,12 @@ final class OrphanVisitsListFiltering extends OrphanVisitsCountFiltering DateRange|null $dateRange = null, bool $excludeBots = false, ApiKey|null $apiKey = null, + string|null $domain = null, OrphanVisitType|null $type = null, + string $defaultDomain = '', public readonly int|null $limit = null, public readonly int|null $offset = null, ) { - parent::__construct($dateRange, $excludeBots, $apiKey, $type); + parent::__construct($dateRange, $excludeBots, $apiKey, $domain, $type, $defaultDomain); } } diff --git a/module/Core/src/Visit/Persistence/WithDomainVisitsCountFiltering.php b/module/Core/src/Visit/Persistence/WithDomainVisitsCountFiltering.php new file mode 100644 index 00000000..5d5cc74a --- /dev/null +++ b/module/Core/src/Visit/Persistence/WithDomainVisitsCountFiltering.php @@ -0,0 +1,20 @@ +createVisitsByTagQueryBuilder($tag, $filtering); return $this->resolveVisitsWithNativeQuery($qb, $filtering->limit, $filtering->offset); } - public function countVisitsByTag(string $tag, VisitsCountFiltering $filtering): int + public function countVisitsByTag(string $tag, WithDomainVisitsCountFiltering $filtering): int { $qb = $this->createVisitsByTagQueryBuilder($tag, $filtering); $qb->select('COUNT(v.id)'); @@ -83,19 +83,29 @@ class VisitRepository extends EntitySpecificationRepository implements VisitRepo return (int) $qb->getQuery()->getSingleScalarResult(); } - private function createVisitsByTagQueryBuilder(string $tag, VisitsCountFiltering $filtering): QueryBuilder + private function createVisitsByTagQueryBuilder(string $tag, WithDomainVisitsCountFiltering $filtering): QueryBuilder { + $conn = $this->getEntityManager()->getConnection(); + // Parameters in this query need to be inlined, not bound, as we need to use it as sub-query later. $qb = $this->getEntityManager()->createQueryBuilder(); $qb->from(Visit::class, 'v') ->join('v.shortUrl', 's') ->join('s.tags', 't') - ->where($qb->expr()->eq('t.name', $this->getEntityManager()->getConnection()->quote($tag))); + ->where($qb->expr()->eq('t.name', $conn->quote($tag))); if ($filtering->excludeBots) { $qb->andWhere($qb->expr()->eq('v.potentialBot', 'false')); } + $domain = $filtering->domain; + if ($domain === Domain::DEFAULT_AUTHORITY) { + $qb->andWhere($qb->expr()->isNull('s.domain')); + } elseif ($domain !== null) { + $qb->join('s.domain', 'd') + ->andWhere($qb->expr()->eq('d.authority', $conn->quote($domain))); + } + $this->applyDatesInline($qb, $filtering->dateRange); $this->applySpecification($qb, $filtering->apiKey?->inlinedSpec(), 'v'); @@ -149,15 +159,7 @@ class VisitRepository extends EntitySpecificationRepository implements VisitRepo return []; } - $qb = $this->createAllVisitsQueryBuilder($filtering); - $qb->andWhere($qb->expr()->isNull('v.shortUrl')); - - // Parameters in this query need to be inlined, not bound, as we need to use it as sub-query later - if ($filtering->type) { - $conn = $this->getEntityManager()->getConnection(); - $qb->andWhere($qb->expr()->eq('v.type', $conn->quote($filtering->type->value))); - } - + $qb = $this->createOrphanVisitsQueryBuilder($filtering); return $this->resolveVisitsWithNativeQuery($qb, $filtering->limit, $filtering->offset); } @@ -167,36 +169,78 @@ class VisitRepository extends EntitySpecificationRepository implements VisitRepo return 0; } - return (int) $this->matchSingleScalarResult(new CountOfOrphanVisits($filtering)); + $qb = $this->createOrphanVisitsQueryBuilder($filtering); + $qb->select('COUNT(v.id)'); + + return (int) $qb->getQuery()->getSingleScalarResult(); + } + + private function createOrphanVisitsQueryBuilder(OrphanVisitsCountFiltering $filtering): QueryBuilder + { + $qb = $this->createAllVisitsQueryBuilder($filtering); + $qb->andWhere($qb->expr()->isNull('v.shortUrl')); + + // Parameters in this query need to be inlined, not bound, as we need to use it as sub-query later + $conn = $this->getEntityManager()->getConnection(); + + if ($filtering->type) { + $qb->andWhere($qb->expr()->eq('v.type', $conn->quote($filtering->type->value))); + } + + $domain = $filtering->domain; + $domain = $domain === Domain::DEFAULT_AUTHORITY ? $filtering->defaultDomain : $domain; + if ($domain !== null) { + $qb->andWhere($qb->expr()->like('v.visitedUrl', $conn->quote('%' . $domain . '%'))); + } + + return $qb; } /** * @return Visit[] */ - public function findNonOrphanVisits(VisitsListFiltering $filtering): array + public function findNonOrphanVisits(WithDomainVisitsListFiltering $filtering): array { + $qb = $this->createNonOrphanVisitsQueryBuilder($filtering); + return $this->resolveVisitsWithNativeQuery($qb, $filtering->limit, $filtering->offset); + } + + public function countNonOrphanVisits(WithDomainVisitsCountFiltering $filtering): int + { + $qb = $this->createNonOrphanVisitsQueryBuilder($filtering); + $qb->select('COUNT(v.id)'); + + return (int) $qb->getQuery()->getSingleScalarResult(); + } + + private function createNonOrphanVisitsQueryBuilder(WithDomainVisitsCountFiltering $filtering): QueryBuilder + { + $conn = $this->getEntityManager()->getConnection(); $qb = $this->createAllVisitsQueryBuilder($filtering); $qb->andWhere($qb->expr()->isNotNull('v.shortUrl')); $apiKey = $filtering->apiKey; - if (ApiKey::isShortUrlRestricted($apiKey)) { + $domain = $filtering->domain; + if (ApiKey::isShortUrlRestricted($apiKey) || $domain !== null) { $qb->join('v.shortUrl', 's'); } + if ($domain === Domain::DEFAULT_AUTHORITY) { + $qb->andWhere($qb->expr()->isNull('s.domain')); + } elseif ($domain !== null) { + $qb->join('s.domain', 'd') + ->andWhere($qb->expr()->eq('d.authority', $conn->quote($domain))); + } + $this->applySpecification($qb, $apiKey?->inlinedSpec(), 'v'); - return $this->resolveVisitsWithNativeQuery($qb, $filtering->limit, $filtering->offset); + return $qb; } - public function countNonOrphanVisits(VisitsCountFiltering $filtering): int + private function createAllVisitsQueryBuilder(VisitsCountFiltering $filtering): QueryBuilder { - return (int) $this->matchSingleScalarResult(new CountOfNonOrphanVisits($filtering)); - } - - private function createAllVisitsQueryBuilder(VisitsListFiltering|OrphanVisitsListFiltering $filtering): QueryBuilder - { - // Parameters in this query need to be inlined, not bound, as we need to use it as sub-query later - // Since they are not provided by the caller, it's reasonably safe + // Parameters in this query need to be inlined, not bound, as we need to use it as sub-query later. + // Since they are not provided by the caller, it's reasonably safe. $qb = $this->getEntityManager()->createQueryBuilder(); $qb->from(Visit::class, 'v'); diff --git a/module/Core/src/Visit/Repository/VisitRepositoryInterface.php b/module/Core/src/Visit/Repository/VisitRepositoryInterface.php index b5590f8c..ca97d12e 100644 --- a/module/Core/src/Visit/Repository/VisitRepositoryInterface.php +++ b/module/Core/src/Visit/Repository/VisitRepositoryInterface.php @@ -12,6 +12,8 @@ use Shlinkio\Shlink\Core\Visit\Persistence\OrphanVisitsCountFiltering; use Shlinkio\Shlink\Core\Visit\Persistence\OrphanVisitsListFiltering; use Shlinkio\Shlink\Core\Visit\Persistence\VisitsCountFiltering; use Shlinkio\Shlink\Core\Visit\Persistence\VisitsListFiltering; +use Shlinkio\Shlink\Core\Visit\Persistence\WithDomainVisitsCountFiltering; +use Shlinkio\Shlink\Core\Visit\Persistence\WithDomainVisitsListFiltering; /** * @extends ObjectRepository @@ -28,9 +30,9 @@ interface VisitRepositoryInterface extends ObjectRepository, EntitySpecification /** * @return Visit[] */ - public function findVisitsByTag(string $tag, VisitsListFiltering $filtering): array; + public function findVisitsByTag(string $tag, WithDomainVisitsListFiltering $filtering): array; - public function countVisitsByTag(string $tag, VisitsCountFiltering $filtering): int; + public function countVisitsByTag(string $tag, WithDomainVisitsCountFiltering $filtering): int; /** * @return Visit[] @@ -49,9 +51,9 @@ interface VisitRepositoryInterface extends ObjectRepository, EntitySpecification /** * @return Visit[] */ - public function findNonOrphanVisits(VisitsListFiltering $filtering): array; + public function findNonOrphanVisits(WithDomainVisitsListFiltering $filtering): array; - public function countNonOrphanVisits(VisitsCountFiltering $filtering): int; + public function countNonOrphanVisits(WithDomainVisitsCountFiltering $filtering): int; public function findMostRecentOrphanVisit(): Visit|null; } diff --git a/module/Core/src/Visit/Spec/CountOfNonOrphanVisits.php b/module/Core/src/Visit/Spec/CountOfNonOrphanVisits.php deleted file mode 100644 index d81cd21b..00000000 --- a/module/Core/src/Visit/Spec/CountOfNonOrphanVisits.php +++ /dev/null @@ -1,39 +0,0 @@ -filtering->dateRange), - ]; - - if ($this->filtering->excludeBots) { - $conditions[] = Spec::eq('potentialBot', false); - } - - $apiKey = $this->filtering->apiKey; - if ($apiKey !== null) { - $conditions[] = new WithApiKeySpecsEnsuringJoin($apiKey, 'shortUrl'); - } - - return Spec::countOf(Spec::andX(...$conditions)); - } -} diff --git a/module/Core/src/Visit/Spec/CountOfOrphanVisits.php b/module/Core/src/Visit/Spec/CountOfOrphanVisits.php deleted file mode 100644 index 9d9cab56..00000000 --- a/module/Core/src/Visit/Spec/CountOfOrphanVisits.php +++ /dev/null @@ -1,37 +0,0 @@ -filtering->dateRange), - ]; - - if ($this->filtering->excludeBots) { - $conditions[] = Spec::eq('potentialBot', false); - } - - if ($this->filtering->type) { - $conditions[] = Spec::eq('type', $this->filtering->type->value); - } - - return Spec::countOf(Spec::andX(...$conditions)); - } -} diff --git a/module/Core/src/Visit/VisitsStatsHelper.php b/module/Core/src/Visit/VisitsStatsHelper.php index 412decc7..b9721f3c 100644 --- a/module/Core/src/Visit/VisitsStatsHelper.php +++ b/module/Core/src/Visit/VisitsStatsHelper.php @@ -7,6 +7,7 @@ namespace Shlinkio\Shlink\Core\Visit; use Doctrine\ORM\EntityManagerInterface; use Pagerfanta\Adapter\AdapterInterface; use Shlinkio\Shlink\Common\Paginator\Paginator; +use Shlinkio\Shlink\Core\Config\Options\UrlShortenerOptions; use Shlinkio\Shlink\Core\Domain\Entity\Domain; use Shlinkio\Shlink\Core\Domain\Repository\DomainRepository; use Shlinkio\Shlink\Core\Exception\DomainNotFoundException; @@ -23,6 +24,7 @@ use Shlinkio\Shlink\Core\Visit\Entity\Visit; use Shlinkio\Shlink\Core\Visit\Model\OrphanVisitsParams; use Shlinkio\Shlink\Core\Visit\Model\VisitsParams; use Shlinkio\Shlink\Core\Visit\Model\VisitsStats; +use Shlinkio\Shlink\Core\Visit\Model\WithDomainVisitsParams; use Shlinkio\Shlink\Core\Visit\Paginator\Adapter\DomainVisitsPaginatorAdapter; use Shlinkio\Shlink\Core\Visit\Paginator\Adapter\NonOrphanVisitsPaginatorAdapter; use Shlinkio\Shlink\Core\Visit\Paginator\Adapter\OrphanVisitsPaginatorAdapter; @@ -37,7 +39,7 @@ use Shlinkio\Shlink\Rest\Entity\ApiKey; readonly class VisitsStatsHelper implements VisitsStatsHelperInterface { - public function __construct(private EntityManagerInterface $em) + public function __construct(private EntityManagerInterface $em, private UrlShortenerOptions $options) { } @@ -88,7 +90,7 @@ readonly class VisitsStatsHelper implements VisitsStatsHelperInterface /** * @inheritDoc */ - public function visitsForTag(string $tag, VisitsParams $params, ApiKey|null $apiKey = null): Paginator + public function visitsForTag(string $tag, WithDomainVisitsParams $params, ApiKey|null $apiKey = null): Paginator { /** @var TagRepository $tagRepo */ $tagRepo = $this->em->getRepository(Tag::class); @@ -127,10 +129,13 @@ readonly class VisitsStatsHelper implements VisitsStatsHelperInterface /** @var VisitRepository $repo */ $repo = $this->em->getRepository(Visit::class); - return $this->createPaginator(new OrphanVisitsPaginatorAdapter($repo, $params, $apiKey), $params); + return $this->createPaginator( + new OrphanVisitsPaginatorAdapter($repo, $params, $apiKey, $this->options), + $params, + ); } - public function nonOrphanVisits(VisitsParams $params, ApiKey|null $apiKey = null): Paginator + public function nonOrphanVisits(WithDomainVisitsParams $params, ApiKey|null $apiKey = null): Paginator { /** @var VisitRepository $repo */ $repo = $this->em->getRepository(Visit::class); diff --git a/module/Core/src/Visit/VisitsStatsHelperInterface.php b/module/Core/src/Visit/VisitsStatsHelperInterface.php index 12e58933..418eae61 100644 --- a/module/Core/src/Visit/VisitsStatsHelperInterface.php +++ b/module/Core/src/Visit/VisitsStatsHelperInterface.php @@ -13,6 +13,7 @@ use Shlinkio\Shlink\Core\Visit\Entity\Visit; use Shlinkio\Shlink\Core\Visit\Model\OrphanVisitsParams; use Shlinkio\Shlink\Core\Visit\Model\VisitsParams; use Shlinkio\Shlink\Core\Visit\Model\VisitsStats; +use Shlinkio\Shlink\Core\Visit\Model\WithDomainVisitsParams; use Shlinkio\Shlink\Rest\Entity\ApiKey; interface VisitsStatsHelperInterface @@ -33,7 +34,7 @@ interface VisitsStatsHelperInterface * @return Paginator * @throws TagNotFoundException */ - public function visitsForTag(string $tag, VisitsParams $params, ApiKey|null $apiKey = null): Paginator; + public function visitsForTag(string $tag, WithDomainVisitsParams $params, ApiKey|null $apiKey = null): Paginator; /** * @return Paginator @@ -49,5 +50,5 @@ interface VisitsStatsHelperInterface /** * @return Paginator */ - public function nonOrphanVisits(VisitsParams $params, ApiKey|null $apiKey = null): Paginator; + public function nonOrphanVisits(WithDomainVisitsParams $params, ApiKey|null $apiKey = null): Paginator; } diff --git a/module/Core/test-db/ShortUrl/Repository/ShortUrlListRepositoryTest.php b/module/Core/test-db/ShortUrl/Repository/ShortUrlListRepositoryTest.php index 435c3e58..11cbfde1 100644 --- a/module/Core/test-db/ShortUrl/Repository/ShortUrlListRepositoryTest.php +++ b/module/Core/test-db/ShortUrl/Repository/ShortUrlListRepositoryTest.php @@ -22,6 +22,9 @@ use Shlinkio\Shlink\Core\ShortUrl\Repository\ShortUrlListRepository; use Shlinkio\Shlink\Core\ShortUrl\Resolver\PersistenceShortUrlRelationResolver; use Shlinkio\Shlink\Core\Visit\Entity\Visit; use Shlinkio\Shlink\Core\Visit\Model\Visitor; +use Shlinkio\Shlink\Rest\ApiKey\Model\ApiKeyMeta; +use Shlinkio\Shlink\Rest\ApiKey\Model\RoleDefinition; +use Shlinkio\Shlink\Rest\Entity\ApiKey; use Shlinkio\Shlink\TestUtils\DbTest\DatabaseTestCase; use function array_map; @@ -82,7 +85,6 @@ class ShortUrlListRepositoryTest extends DatabaseTestCase $foo2->setVisits(new ArrayCollection($visits2)); $ref = new ReflectionObject($foo2); $dateProp = $ref->getProperty('dateCreated'); - $dateProp->setAccessible(true); $dateProp->setValue($foo2, Chronos::now()->subDays(5)); $this->getEntityManager()->persist($foo2); @@ -239,6 +241,24 @@ class ShortUrlListRepositoryTest extends DatabaseTestCase self::assertEquals(0, $this->repo->countList( new ShortUrlsCountFiltering(tags: ['foo', 'bar', 'baz'], tagsMode: TagsMode::ALL), )); + + self::assertEquals(2, $this->repo->countList(new ShortUrlsCountFiltering(excludeTags: ['foo']))); + self::assertEquals(0, $this->repo->countList(new ShortUrlsCountFiltering(excludeTags: ['foo', 'bar']))); + self::assertEquals(4, $this->repo->countList(new ShortUrlsCountFiltering( + excludeTags: ['foo', 'bar'], + excludeTagsMode: TagsMode::ALL, + ))); + + self::assertEquals(2, $this->repo->countList(new ShortUrlsCountFiltering(tags: ['foo'], excludeTags: ['bar']))); + self::assertEquals(1, $this->repo->countList(new ShortUrlsCountFiltering( + tags: ['foo'], + excludeTags: ['bar', 'baz'], + ))); + self::assertEquals(3, $this->repo->countList(new ShortUrlsCountFiltering( + tags: ['foo'], + excludeTags: ['bar', 'baz'], + excludeTagsMode: TagsMode::ALL, + ))); } #[Test] @@ -349,4 +369,70 @@ class ShortUrlListRepositoryTest extends DatabaseTestCase excludePastValidUntil: true, ))); } + + #[Test] + public function filteringByApiKeyNameIsPossible(): void + { + $apiKey1 = ApiKey::create(); + $this->getEntityManager()->persist($apiKey1); + $apiKey2 = ApiKey::fromMeta(ApiKeyMeta::withRoles(RoleDefinition::forAuthoredShortUrls())); + $this->getEntityManager()->persist($apiKey2); + $apiKey3 = ApiKey::create(); + $this->getEntityManager()->persist($apiKey3); + + $shortUrl1 = ShortUrl::create(ShortUrlCreation::fromRawData([ + 'longUrl' => 'https://foo1', + 'apiKey' => $apiKey1, + ]), $this->relationResolver); + $this->getEntityManager()->persist($shortUrl1); + $shortUrl2 = ShortUrl::create(ShortUrlCreation::fromRawData([ + 'longUrl' => 'https://foo2', + 'apiKey' => $apiKey1, + ]), $this->relationResolver); + $this->getEntityManager()->persist($shortUrl2); + $shortUrl3 = ShortUrl::create(ShortUrlCreation::fromRawData([ + 'longUrl' => 'https://foo3', + 'apiKey' => $apiKey2, + ]), $this->relationResolver); + $this->getEntityManager()->persist($shortUrl3); + $shortUrl4 = ShortUrl::create(ShortUrlCreation::fromRawData([ + 'longUrl' => 'https://foo4', + 'apiKey' => $apiKey1, + ]), $this->relationResolver); + $this->getEntityManager()->persist($shortUrl4); + + $this->getEntityManager()->flush(); + + // It is possible to filter by API key name when no API key or ADMIN API key is provided + self::assertCount(3, $this->repo->findList(new ShortUrlsListFiltering(apiKeyName: $apiKey1->name))); + self::assertCount(1, $this->repo->findList(new ShortUrlsListFiltering(apiKeyName: $apiKey2->name))); + self::assertCount(0, $this->repo->findList(new ShortUrlsListFiltering(apiKeyName: $apiKey3->name))); + + self::assertCount(3, $this->repo->findList(new ShortUrlsListFiltering( + apiKey: $apiKey1, + apiKeyName: $apiKey1->name, + ))); + self::assertCount(1, $this->repo->findList(new ShortUrlsListFiltering( + apiKey: $apiKey1, + apiKeyName: $apiKey2->name, + ))); + self::assertCount(0, $this->repo->findList(new ShortUrlsListFiltering( + apiKey: $apiKey1, + apiKeyName: $apiKey3->name, + ))); + + // When a non-admin API key is passed, it allows to filter by itself only + self::assertCount(1, $this->repo->findList(new ShortUrlsListFiltering( + apiKey: $apiKey2, + apiKeyName: $apiKey1->name, // Ignored. Only API key 2 results are returned + ))); + self::assertCount(1, $this->repo->findList(new ShortUrlsListFiltering( + apiKey: $apiKey2, + apiKeyName: $apiKey2->name, + ))); + self::assertCount(1, $this->repo->findList(new ShortUrlsListFiltering( + apiKey: $apiKey2, + apiKeyName: $apiKey3->name, // Ignored. Only API key 2 results are returned + ))); + } } diff --git a/module/Core/test-db/Visit/Repository/VisitRepositoryTest.php b/module/Core/test-db/Visit/Repository/VisitRepositoryTest.php index fe53629f..56b6175f 100644 --- a/module/Core/test-db/Visit/Repository/VisitRepositoryTest.php +++ b/module/Core/test-db/Visit/Repository/VisitRepositoryTest.php @@ -22,6 +22,8 @@ use Shlinkio\Shlink\Core\Visit\Persistence\OrphanVisitsCountFiltering; use Shlinkio\Shlink\Core\Visit\Persistence\OrphanVisitsListFiltering; use Shlinkio\Shlink\Core\Visit\Persistence\VisitsCountFiltering; use Shlinkio\Shlink\Core\Visit\Persistence\VisitsListFiltering; +use Shlinkio\Shlink\Core\Visit\Persistence\WithDomainVisitsCountFiltering; +use Shlinkio\Shlink\Core\Visit\Persistence\WithDomainVisitsListFiltering; use Shlinkio\Shlink\Core\Visit\Repository\OrphanVisitsCountRepository; use Shlinkio\Shlink\Core\Visit\Repository\ShortUrlVisitsCountRepository; use Shlinkio\Shlink\Core\Visit\Repository\VisitRepository; @@ -187,13 +189,13 @@ class VisitRepositoryTest extends DatabaseTestCase $this->createShortUrlsAndVisits(false, [$foo]); $this->getEntityManager()->flush(); - self::assertCount(0, $this->repo->findVisitsByTag('invalid', new VisitsListFiltering())); - self::assertCount(18, $this->repo->findVisitsByTag($foo, new VisitsListFiltering())); - self::assertCount(12, $this->repo->findVisitsByTag($foo, new VisitsListFiltering(null, true))); - self::assertCount(6, $this->repo->findVisitsByTag($foo, new VisitsListFiltering( + self::assertCount(0, $this->repo->findVisitsByTag('invalid', new WithDomainVisitsListFiltering())); + self::assertCount(18, $this->repo->findVisitsByTag($foo, new WithDomainVisitsListFiltering())); + self::assertCount(12, $this->repo->findVisitsByTag($foo, new WithDomainVisitsListFiltering(null, true))); + self::assertCount(6, $this->repo->findVisitsByTag($foo, new WithDomainVisitsListFiltering( DateRange::between(Chronos::parse('2016-01-02'), Chronos::parse('2016-01-03')), ))); - self::assertCount(12, $this->repo->findVisitsByTag($foo, new VisitsListFiltering( + self::assertCount(12, $this->repo->findVisitsByTag($foo, new WithDomainVisitsListFiltering( DateRange::since(Chronos::parse('2016-01-03')), ))); } @@ -203,21 +205,40 @@ class VisitRepositoryTest extends DatabaseTestCase { $foo = 'foo'; - $this->createShortUrlsAndVisits(false, [$foo]); + $shortUrl1 = ShortUrl::create(ShortUrlCreation::fromRawData([ + ShortUrlInputFilter::LONG_URL => 'https://longUrl', + ShortUrlInputFilter::TAGS => [$foo], + ShortUrlInputFilter::DOMAIN => 'foo.com', + ]), $this->relationResolver); + $this->getEntityManager()->persist($shortUrl1); + $this->createVisitsForShortUrl($shortUrl1, 6); + + $shortUrl2 = ShortUrl::create(ShortUrlCreation::fromRawData([ + ShortUrlInputFilter::LONG_URL => 'https://longUrl', + ShortUrlInputFilter::TAGS => [$foo], + ]), $this->relationResolver); + $this->getEntityManager()->persist($shortUrl2); + $this->createVisitsForShortUrl($shortUrl2, 6); + $this->getEntityManager()->flush(); - $this->createShortUrlsAndVisits(false, [$foo]); - $this->getEntityManager()->flush(); - - self::assertEquals(0, $this->repo->countVisitsByTag('invalid', new VisitsCountFiltering())); - self::assertEquals(12, $this->repo->countVisitsByTag($foo, new VisitsCountFiltering())); - self::assertEquals(8, $this->repo->countVisitsByTag($foo, new VisitsCountFiltering(null, true))); - self::assertEquals(4, $this->repo->countVisitsByTag($foo, new VisitsCountFiltering( + self::assertEquals(0, $this->repo->countVisitsByTag('invalid', new WithDomainVisitsCountFiltering())); + self::assertEquals(12, $this->repo->countVisitsByTag($foo, new WithDomainVisitsCountFiltering())); + self::assertEquals(8, $this->repo->countVisitsByTag($foo, new WithDomainVisitsCountFiltering( + excludeBots: true, + ))); + self::assertEquals(4, $this->repo->countVisitsByTag($foo, new WithDomainVisitsCountFiltering( DateRange::between(Chronos::parse('2016-01-02'), Chronos::parse('2016-01-03')), ))); - self::assertEquals(8, $this->repo->countVisitsByTag($foo, new VisitsCountFiltering( + self::assertEquals(8, $this->repo->countVisitsByTag($foo, new WithDomainVisitsCountFiltering( DateRange::since(Chronos::parse('2016-01-03')), ))); + self::assertEquals(6, $this->repo->countVisitsByTag($foo, new WithDomainVisitsCountFiltering( + domain: 'foo.com', + ))); + self::assertEquals(6, $this->repo->countVisitsByTag($foo, new WithDomainVisitsCountFiltering( + domain: Domain::DEFAULT_AUTHORITY, + ))); } #[Test] @@ -318,13 +339,17 @@ class VisitRepositoryTest extends DatabaseTestCase $this->getEntityManager()->flush(); - self::assertEquals(4 + 5 + 7, $this->repo->countNonOrphanVisits(new VisitsCountFiltering())); + self::assertEquals(4 + 5 + 7, $this->repo->countNonOrphanVisits(new WithDomainVisitsCountFiltering())); self::assertEquals(4 + 5 + 7, $this->countRepo->countNonOrphanVisits(new VisitsCountFiltering())); - self::assertEquals(4, $this->repo->countNonOrphanVisits(new VisitsCountFiltering(apiKey: $apiKey1))); + self::assertEquals(4, $this->repo->countNonOrphanVisits(new WithDomainVisitsCountFiltering(apiKey: $apiKey1))); self::assertEquals(4, $this->countRepo->countNonOrphanVisits(new VisitsCountFiltering(apiKey: $apiKey1))); - self::assertEquals(5 + 7, $this->repo->countNonOrphanVisits(new VisitsCountFiltering(apiKey: $apiKey2))); + self::assertEquals(5 + 7, $this->repo->countNonOrphanVisits(new WithDomainVisitsCountFiltering( + apiKey: $apiKey2, + ))); self::assertEquals(5 + 7, $this->countRepo->countNonOrphanVisits(new VisitsCountFiltering(apiKey: $apiKey2))); - self::assertEquals(4 + 7, $this->repo->countNonOrphanVisits(new VisitsCountFiltering(apiKey: $domainApiKey))); + self::assertEquals(4 + 7, $this->repo->countNonOrphanVisits(new WithDomainVisitsCountFiltering( + apiKey: $domainApiKey, + ))); self::assertEquals(4 + 7, $this->countRepo->countNonOrphanVisits(new VisitsCountFiltering( apiKey: $domainApiKey, ))); @@ -334,21 +359,27 @@ class VisitRepositoryTest extends DatabaseTestCase self::assertEquals(0, $this->orphanCountRepo->countOrphanVisits(new OrphanVisitsCountFiltering( apiKey: $noOrphanVisitsApiKey, ))); - self::assertEquals(4, $this->repo->countNonOrphanVisits(new VisitsCountFiltering(DateRange::since( + self::assertEquals(4, $this->repo->countNonOrphanVisits(new WithDomainVisitsCountFiltering(DateRange::since( Chronos::parse('2016-01-05')->startOfDay(), )))); - self::assertEquals(2, $this->repo->countNonOrphanVisits(new VisitsCountFiltering(DateRange::since( + self::assertEquals(2, $this->repo->countNonOrphanVisits(new WithDomainVisitsCountFiltering(DateRange::since( Chronos::parse('2016-01-03')->startOfDay(), ), false, $apiKey1))); - self::assertEquals(1, $this->repo->countNonOrphanVisits(new VisitsCountFiltering(DateRange::since( + self::assertEquals(1, $this->repo->countNonOrphanVisits(new WithDomainVisitsCountFiltering(DateRange::since( Chronos::parse('2016-01-07')->startOfDay(), ), false, $apiKey2))); self::assertEquals(3 + 5, $this->repo->countNonOrphanVisits( - new VisitsCountFiltering(excludeBots: true, apiKey: $apiKey2), + new WithDomainVisitsCountFiltering(excludeBots: true, apiKey: $apiKey2), )); self::assertEquals(3 + 5, $this->countRepo->countNonOrphanVisits( new VisitsCountFiltering(excludeBots: true, apiKey: $apiKey2), )); + self::assertEquals(4 + 7, $this->repo->countNonOrphanVisits( + new WithDomainVisitsCountFiltering(domain: $domain->authority), + )); + self::assertEquals(5, $this->repo->countNonOrphanVisits( + new WithDomainVisitsCountFiltering(domain: Domain::DEFAULT_AUTHORITY), + )); self::assertEquals(4, $this->repo->countOrphanVisits(new OrphanVisitsCountFiltering())); self::assertEquals(4, $this->orphanCountRepo->countOrphanVisits(new OrphanVisitsCountFiltering())); self::assertEquals(3, $this->repo->countOrphanVisits(new OrphanVisitsCountFiltering(excludeBots: true))); @@ -374,11 +405,11 @@ class VisitRepositoryTest extends DatabaseTestCase Chronos::parse(sprintf('2020-01-0%s', $i + 1)), )); $this->getEntityManager()->persist($this->setDateOnVisit( - fn () => Visit::forInvalidShortUrl(Visitor::empty()), + fn () => Visit::forInvalidShortUrl(Visitor::fromParams(visitedUrl: 'https://s.test/bar')), Chronos::parse(sprintf('2020-01-0%s', $i + 1)), )); $this->getEntityManager()->persist($this->setDateOnVisit( - fn () => Visit::forRegularNotFound(Visitor::empty()), + fn () => Visit::forRegularNotFound(Visitor::fromParams(visitedUrl: 'https://example.com/foo?1=2')), Chronos::parse(sprintf('2020-01-0%s', $i + 1)), )); @@ -417,6 +448,11 @@ class VisitRepositoryTest extends DatabaseTestCase type: OrphanVisitType::BASE_URL, limit: 4, ))); + self::assertCount(6, $this->repo->findOrphanVisits(new OrphanVisitsListFiltering(domain: 'example.com'))); + self::assertCount(6, $this->repo->findOrphanVisits(new OrphanVisitsListFiltering( + domain: Domain::DEFAULT_AUTHORITY, + defaultDomain: 's.test', + ))); } #[Test] @@ -432,11 +468,11 @@ class VisitRepositoryTest extends DatabaseTestCase Chronos::parse(sprintf('2020-01-0%s', $i + 1)), )); $this->getEntityManager()->persist($this->setDateOnVisit( - fn () => Visit::forInvalidShortUrl(Visitor::empty()), + fn () => Visit::forInvalidShortUrl(Visitor::fromParams(visitedUrl: 'https://s.test/foo/bar')), Chronos::parse(sprintf('2020-01-0%s', $i + 1)), )); $this->getEntityManager()->persist($this->setDateOnVisit( - fn () => Visit::forRegularNotFound(Visitor::empty()), + fn () => Visit::forRegularNotFound(Visitor::fromParams(visitedUrl: 'https://example.com/foo/bar')), Chronos::parse(sprintf('2020-01-0%s', $i + 1)), )); } @@ -465,6 +501,11 @@ class VisitRepositoryTest extends DatabaseTestCase self::assertEquals(6, $this->repo->countOrphanVisits(new OrphanVisitsCountFiltering( type: OrphanVisitType::REGULAR_404, ))); + self::assertEquals(6, $this->repo->countOrphanVisits(new OrphanVisitsCountFiltering(domain: 'example.com'))); + self::assertEquals(6, $this->repo->countOrphanVisits(new OrphanVisitsCountFiltering( + domain: Domain::DEFAULT_AUTHORITY, + defaultDomain: 's.test', + ))); } #[Test] @@ -479,31 +520,38 @@ class VisitRepositoryTest extends DatabaseTestCase $this->getEntityManager()->flush(); - self::assertCount(21, $this->repo->findNonOrphanVisits(new VisitsListFiltering())); - self::assertCount(21, $this->repo->findNonOrphanVisits(new VisitsListFiltering(DateRange::allTime()))); - self::assertCount(4, $this->repo->findNonOrphanVisits(new VisitsListFiltering(apiKey: $authoredApiKey))); - self::assertCount(7, $this->repo->findNonOrphanVisits(new VisitsListFiltering(DateRange::since( + self::assertCount(21, $this->repo->findNonOrphanVisits(new WithDomainVisitsListFiltering())); + self::assertCount(21, $this->repo->findNonOrphanVisits(new WithDomainVisitsListFiltering( + DateRange::allTime(), + ))); + self::assertCount(4, $this->repo->findNonOrphanVisits(new WithDomainVisitsListFiltering( + apiKey: $authoredApiKey, + ))); + self::assertCount(7, $this->repo->findNonOrphanVisits(new WithDomainVisitsListFiltering(DateRange::since( Chronos::parse('2016-01-05')->endOfDay(), )))); - self::assertCount(12, $this->repo->findNonOrphanVisits(new VisitsListFiltering(DateRange::until( + self::assertCount(12, $this->repo->findNonOrphanVisits(new WithDomainVisitsListFiltering(DateRange::until( Chronos::parse('2016-01-04')->endOfDay(), )))); - self::assertCount(6, $this->repo->findNonOrphanVisits(new VisitsListFiltering(DateRange::between( + self::assertCount(6, $this->repo->findNonOrphanVisits(new WithDomainVisitsListFiltering(DateRange::between( Chronos::parse('2016-01-03')->startOfDay(), Chronos::parse('2016-01-04')->endOfDay(), )))); - self::assertCount(13, $this->repo->findNonOrphanVisits(new VisitsListFiltering(DateRange::between( + self::assertCount(13, $this->repo->findNonOrphanVisits(new WithDomainVisitsListFiltering(DateRange::between( Chronos::parse('2016-01-03')->startOfDay(), Chronos::parse('2016-01-08')->endOfDay(), )))); - self::assertCount(3, $this->repo->findNonOrphanVisits(new VisitsListFiltering(DateRange::between( + self::assertCount(3, $this->repo->findNonOrphanVisits(new WithDomainVisitsListFiltering(DateRange::between( Chronos::parse('2016-01-03')->startOfDay(), Chronos::parse('2016-01-08')->endOfDay(), ), limit: 10, offset: 10))); - self::assertCount(15, $this->repo->findNonOrphanVisits(new VisitsListFiltering(excludeBots: true))); - self::assertCount(10, $this->repo->findNonOrphanVisits(new VisitsListFiltering(limit: 10))); - self::assertCount(1, $this->repo->findNonOrphanVisits(new VisitsListFiltering(limit: 10, offset: 20))); - self::assertCount(5, $this->repo->findNonOrphanVisits(new VisitsListFiltering(limit: 5, offset: 5))); + self::assertCount(15, $this->repo->findNonOrphanVisits(new WithDomainVisitsListFiltering(excludeBots: true))); + self::assertCount(10, $this->repo->findNonOrphanVisits(new WithDomainVisitsListFiltering(limit: 10))); + self::assertCount(1, $this->repo->findNonOrphanVisits(new WithDomainVisitsListFiltering( + limit: 10, + offset: 20, + ))); + self::assertCount(5, $this->repo->findNonOrphanVisits(new WithDomainVisitsListFiltering(limit: 5, offset: 5))); } #[Test] @@ -526,6 +574,7 @@ class VisitRepositoryTest extends DatabaseTestCase /** * @return array{string, string, ShortUrl} + * @fixme This method does too many things and is not intuitive. It should be removed or simplified */ private function createShortUrlsAndVisits( bool|string $withDomain = true, @@ -558,6 +607,10 @@ class VisitRepositoryTest extends DatabaseTestCase return [$shortCode, $domain, $shortUrl]; } + /** + * @param int $amount - How many visits in total. Defaults to 6 + * @param int $botsAmount - How many of the visits should be bots. Defaults to 2 + */ private function createVisitsForShortUrl(ShortUrl $shortUrl, int $amount = 6, int $botsAmount = 2): void { for ($i = 0; $i < $amount; $i++) { diff --git a/module/Core/test/Visit/Paginator/Adapter/NonOrphanVisitsPaginatorAdapterTest.php b/module/Core/test/Visit/Paginator/Adapter/NonOrphanVisitsPaginatorAdapterTest.php index 2dbaa25a..d021646d 100644 --- a/module/Core/test/Visit/Paginator/Adapter/NonOrphanVisitsPaginatorAdapterTest.php +++ b/module/Core/test/Visit/Paginator/Adapter/NonOrphanVisitsPaginatorAdapterTest.php @@ -10,10 +10,10 @@ use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Shlinkio\Shlink\Core\Visit\Entity\Visit; use Shlinkio\Shlink\Core\Visit\Model\Visitor; -use Shlinkio\Shlink\Core\Visit\Model\VisitsParams; +use Shlinkio\Shlink\Core\Visit\Model\WithDomainVisitsParams; use Shlinkio\Shlink\Core\Visit\Paginator\Adapter\NonOrphanVisitsPaginatorAdapter; -use Shlinkio\Shlink\Core\Visit\Persistence\VisitsCountFiltering; -use Shlinkio\Shlink\Core\Visit\Persistence\VisitsListFiltering; +use Shlinkio\Shlink\Core\Visit\Persistence\WithDomainVisitsCountFiltering; +use Shlinkio\Shlink\Core\Visit\Persistence\WithDomainVisitsListFiltering; use Shlinkio\Shlink\Core\Visit\Repository\VisitRepositoryInterface; use Shlinkio\Shlink\Rest\Entity\ApiKey; @@ -21,13 +21,13 @@ class NonOrphanVisitsPaginatorAdapterTest extends TestCase { private NonOrphanVisitsPaginatorAdapter $adapter; private MockObject & VisitRepositoryInterface $repo; - private VisitsParams $params; + private WithDomainVisitsParams $params; private ApiKey $apiKey; protected function setUp(): void { $this->repo = $this->createMock(VisitRepositoryInterface::class); - $this->params = VisitsParams::fromRawData([]); + $this->params = WithDomainVisitsParams::fromRawData([]); $this->apiKey = ApiKey::create(); $this->adapter = new NonOrphanVisitsPaginatorAdapter($this->repo, $this->params, $this->apiKey); @@ -38,7 +38,7 @@ class NonOrphanVisitsPaginatorAdapterTest extends TestCase { $expectedCount = 5; $this->repo->expects($this->once())->method('countNonOrphanVisits')->with( - new VisitsCountFiltering($this->params->dateRange, $this->params->excludeBots, $this->apiKey), + new WithDomainVisitsCountFiltering($this->params->dateRange, $this->params->excludeBots, $this->apiKey), )->willReturn($expectedCount); $result = $this->adapter->getNbResults(); @@ -55,12 +55,12 @@ class NonOrphanVisitsPaginatorAdapterTest extends TestCase { $visitor = Visitor::empty(); $list = [Visit::forRegularNotFound($visitor), Visit::forInvalidShortUrl($visitor)]; - $this->repo->expects($this->once())->method('findNonOrphanVisits')->with(new VisitsListFiltering( + $this->repo->expects($this->once())->method('findNonOrphanVisits')->with(new WithDomainVisitsListFiltering( $this->params->dateRange, $this->params->excludeBots, $this->apiKey, - $limit, - $offset, + limit: $limit, + offset: $offset, ))->willReturn($list); $result = $this->adapter->getSlice($offset, $limit); diff --git a/module/Core/test/Visit/Paginator/Adapter/OrphanVisitsPaginatorAdapterTest.php b/module/Core/test/Visit/Paginator/Adapter/OrphanVisitsPaginatorAdapterTest.php index b62fa0c6..b3e800a7 100644 --- a/module/Core/test/Visit/Paginator/Adapter/OrphanVisitsPaginatorAdapterTest.php +++ b/module/Core/test/Visit/Paginator/Adapter/OrphanVisitsPaginatorAdapterTest.php @@ -8,6 +8,7 @@ use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; +use Shlinkio\Shlink\Core\Config\Options\UrlShortenerOptions; use Shlinkio\Shlink\Core\Visit\Entity\Visit; use Shlinkio\Shlink\Core\Visit\Model\OrphanVisitsParams; use Shlinkio\Shlink\Core\Visit\Model\Visitor; @@ -30,7 +31,12 @@ class OrphanVisitsPaginatorAdapterTest extends TestCase $this->params = new OrphanVisitsParams(); $this->apiKey = ApiKey::create(); - $this->adapter = new OrphanVisitsPaginatorAdapter($this->repo, $this->params, $this->apiKey); + $this->adapter = new OrphanVisitsPaginatorAdapter( + $this->repo, + $this->params, + $this->apiKey, + new UrlShortenerOptions(), + ); } #[Test] diff --git a/module/Core/test/Visit/Paginator/Adapter/VisitsForTagPaginatorAdapterTest.php b/module/Core/test/Visit/Paginator/Adapter/VisitsForTagPaginatorAdapterTest.php index c0cd4d0b..cbecd958 100644 --- a/module/Core/test/Visit/Paginator/Adapter/VisitsForTagPaginatorAdapterTest.php +++ b/module/Core/test/Visit/Paginator/Adapter/VisitsForTagPaginatorAdapterTest.php @@ -8,10 +8,10 @@ use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Shlinkio\Shlink\Common\Util\DateRange; -use Shlinkio\Shlink\Core\Visit\Model\VisitsParams; +use Shlinkio\Shlink\Core\Visit\Model\WithDomainVisitsParams; use Shlinkio\Shlink\Core\Visit\Paginator\Adapter\TagVisitsPaginatorAdapter; -use Shlinkio\Shlink\Core\Visit\Persistence\VisitsCountFiltering; -use Shlinkio\Shlink\Core\Visit\Persistence\VisitsListFiltering; +use Shlinkio\Shlink\Core\Visit\Persistence\WithDomainVisitsCountFiltering; +use Shlinkio\Shlink\Core\Visit\Persistence\WithDomainVisitsListFiltering; use Shlinkio\Shlink\Core\Visit\Repository\VisitRepositoryInterface; use Shlinkio\Shlink\Rest\Entity\ApiKey; @@ -33,7 +33,7 @@ class VisitsForTagPaginatorAdapterTest extends TestCase $adapter = $this->createAdapter(null); $this->repo->expects($this->exactly($count))->method('findVisitsByTag')->with( 'foo', - new VisitsListFiltering(DateRange::allTime(), false, null, $limit, $offset), + new WithDomainVisitsListFiltering(DateRange::allTime(), limit: $limit, offset: $offset), )->willReturn([]); for ($i = 0; $i < $count; $i++) { @@ -49,7 +49,7 @@ class VisitsForTagPaginatorAdapterTest extends TestCase $adapter = $this->createAdapter($apiKey); $this->repo->expects($this->once())->method('countVisitsByTag')->with( 'foo', - new VisitsCountFiltering(DateRange::allTime(), false, $apiKey), + new WithDomainVisitsCountFiltering(DateRange::allTime(), apiKey: $apiKey), )->willReturn(3); for ($i = 0; $i < $count; $i++) { @@ -59,6 +59,6 @@ class VisitsForTagPaginatorAdapterTest extends TestCase private function createAdapter(ApiKey|null $apiKey): TagVisitsPaginatorAdapter { - return new TagVisitsPaginatorAdapter($this->repo, 'foo', VisitsParams::fromRawData([]), $apiKey); + return new TagVisitsPaginatorAdapter($this->repo, 'foo', WithDomainVisitsParams::fromRawData([]), $apiKey); } } diff --git a/module/Core/test/Visit/VisitsStatsHelperTest.php b/module/Core/test/Visit/VisitsStatsHelperTest.php index 070ea7e6..68aa4310 100644 --- a/module/Core/test/Visit/VisitsStatsHelperTest.php +++ b/module/Core/test/Visit/VisitsStatsHelperTest.php @@ -12,6 +12,7 @@ use PHPUnit\Framework\Attributes\DataProviderExternal; use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; +use Shlinkio\Shlink\Core\Config\Options\UrlShortenerOptions; use Shlinkio\Shlink\Core\Domain\Entity\Domain; use Shlinkio\Shlink\Core\Domain\Repository\DomainRepository; use Shlinkio\Shlink\Core\Exception\DomainNotFoundException; @@ -29,10 +30,12 @@ use Shlinkio\Shlink\Core\Visit\Model\OrphanVisitsParams; use Shlinkio\Shlink\Core\Visit\Model\Visitor; use Shlinkio\Shlink\Core\Visit\Model\VisitsParams; use Shlinkio\Shlink\Core\Visit\Model\VisitsStats; +use Shlinkio\Shlink\Core\Visit\Model\WithDomainVisitsParams; use Shlinkio\Shlink\Core\Visit\Persistence\OrphanVisitsCountFiltering; use Shlinkio\Shlink\Core\Visit\Persistence\OrphanVisitsListFiltering; use Shlinkio\Shlink\Core\Visit\Persistence\VisitsCountFiltering; -use Shlinkio\Shlink\Core\Visit\Persistence\VisitsListFiltering; +use Shlinkio\Shlink\Core\Visit\Persistence\WithDomainVisitsCountFiltering; +use Shlinkio\Shlink\Core\Visit\Persistence\WithDomainVisitsListFiltering; use Shlinkio\Shlink\Core\Visit\Repository\OrphanVisitsCountRepository; use Shlinkio\Shlink\Core\Visit\Repository\ShortUrlVisitsCountRepository; use Shlinkio\Shlink\Core\Visit\Repository\VisitRepository; @@ -52,7 +55,7 @@ class VisitsStatsHelperTest extends TestCase protected function setUp(): void { $this->em = $this->createMock(EntityManagerInterface::class); - $this->helper = new VisitsStatsHelper($this->em); + $this->helper = new VisitsStatsHelper($this->em, new UrlShortenerOptions()); } #[Test, DataProvider('provideCounts')] @@ -147,7 +150,7 @@ class VisitsStatsHelperTest extends TestCase $this->expectException(TagNotFoundException::class); - $this->helper->visitsForTag($tag, new VisitsParams(), $apiKey); + $this->helper->visitsForTag($tag, new WithDomainVisitsParams(), $apiKey); } #[Test, DataProviderExternal(ApiKeyDataProviders::class, 'adminApiKeysProvider')] @@ -170,7 +173,7 @@ class VisitsStatsHelperTest extends TestCase [Visit::class, $repo2], ]); - $paginator = $this->helper->visitsForTag($tag, new VisitsParams(), $apiKey); + $paginator = $this->helper->visitsForTag($tag, new WithDomainVisitsParams(), $apiKey); self::assertEquals($list, ArrayUtils::iteratorToArray($paginator->getCurrentPageResults())); } @@ -265,14 +268,14 @@ class VisitsStatsHelperTest extends TestCase ); $repo = $this->createMock(VisitRepository::class); $repo->expects($this->once())->method('countNonOrphanVisits')->with( - $this->isInstanceOf(VisitsCountFiltering::class), + $this->isInstanceOf(WithDOmainVisitsCountFiltering::class), )->willReturn(count($list)); $repo->expects($this->once())->method('findNonOrphanVisits')->with( - $this->isInstanceOf(VisitsListFiltering::class), + $this->isInstanceOf(WithDOmainVisitsListFiltering::class), )->willReturn($list); $this->em->expects($this->once())->method('getRepository')->with(Visit::class)->willReturn($repo); - $paginator = $this->helper->nonOrphanVisits(new VisitsParams()); + $paginator = $this->helper->nonOrphanVisits(new WithDomainVisitsParams()); self::assertEquals($list, ArrayUtils::iteratorToArray($paginator->getCurrentPageResults())); } diff --git a/module/Rest/src/Action/Visit/AbstractListVisitsAction.php b/module/Rest/src/Action/Visit/AbstractListVisitsAction.php index b63133fa..6e49c46f 100644 --- a/module/Rest/src/Action/Visit/AbstractListVisitsAction.php +++ b/module/Rest/src/Action/Visit/AbstractListVisitsAction.php @@ -10,7 +10,6 @@ use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; use Shlinkio\Shlink\Common\Paginator\Util\PagerfantaUtils; use Shlinkio\Shlink\Core\Visit\Entity\Visit; -use Shlinkio\Shlink\Core\Visit\Model\VisitsParams; use Shlinkio\Shlink\Core\Visit\VisitsStatsHelperInterface; use Shlinkio\Shlink\Rest\Action\AbstractRestAction; use Shlinkio\Shlink\Rest\Entity\ApiKey; @@ -26,9 +25,8 @@ abstract class AbstractListVisitsAction extends AbstractRestAction public function handle(ServerRequestInterface $request): ResponseInterface { - $params = VisitsParams::fromRawData($request->getQueryParams()); $apiKey = AuthenticationMiddleware::apiKeyFromRequest($request); - $visits = $this->getVisitsPaginator($request, $params, $apiKey); + $visits = $this->getVisitsPaginator($request, $apiKey); return new JsonResponse(['visits' => PagerfantaUtils::serializePaginator($visits)]); } @@ -36,9 +34,5 @@ abstract class AbstractListVisitsAction extends AbstractRestAction /** * @return Pagerfanta */ - abstract protected function getVisitsPaginator( - ServerRequestInterface $request, - VisitsParams $params, - ApiKey $apiKey, - ): Pagerfanta; + abstract protected function getVisitsPaginator(ServerRequestInterface $request, ApiKey $apiKey): Pagerfanta; } diff --git a/module/Rest/src/Action/Visit/DomainVisitsAction.php b/module/Rest/src/Action/Visit/DomainVisitsAction.php index 0e027955..dd1ad292 100644 --- a/module/Rest/src/Action/Visit/DomainVisitsAction.php +++ b/module/Rest/src/Action/Visit/DomainVisitsAction.php @@ -23,8 +23,9 @@ class DomainVisitsAction extends AbstractListVisitsAction parent::__construct($visitsHelper); } - protected function getVisitsPaginator(Request $request, VisitsParams $params, ApiKey $apiKey): Pagerfanta + protected function getVisitsPaginator(Request $request, ApiKey $apiKey): Pagerfanta { + $params = VisitsParams::fromRawData($request->getQueryParams()); $domain = $this->resolveDomainParam($request); return $this->visitsHelper->visitsForDomain($domain, $params, $apiKey); } diff --git a/module/Rest/src/Action/Visit/NonOrphanVisitsAction.php b/module/Rest/src/Action/Visit/NonOrphanVisitsAction.php index b2f7471b..3bcc9929 100644 --- a/module/Rest/src/Action/Visit/NonOrphanVisitsAction.php +++ b/module/Rest/src/Action/Visit/NonOrphanVisitsAction.php @@ -6,18 +6,16 @@ namespace Shlinkio\Shlink\Rest\Action\Visit; use Pagerfanta\Pagerfanta; use Psr\Http\Message\ServerRequestInterface; -use Shlinkio\Shlink\Core\Visit\Model\VisitsParams; +use Shlinkio\Shlink\Core\Visit\Model\WithDomainVisitsParams; use Shlinkio\Shlink\Rest\Entity\ApiKey; class NonOrphanVisitsAction extends AbstractListVisitsAction { protected const string ROUTE_PATH = '/visits/non-orphan'; - protected function getVisitsPaginator( - ServerRequestInterface $request, - VisitsParams $params, - ApiKey $apiKey, - ): Pagerfanta { + protected function getVisitsPaginator(ServerRequestInterface $request, ApiKey $apiKey): Pagerfanta + { + $params = WithDomainVisitsParams::fromRawData($request->getQueryParams()); return $this->visitsHelper->nonOrphanVisits($params, $apiKey); } } diff --git a/module/Rest/src/Action/Visit/OrphanVisitsAction.php b/module/Rest/src/Action/Visit/OrphanVisitsAction.php index b3c246ca..4e1f7745 100644 --- a/module/Rest/src/Action/Visit/OrphanVisitsAction.php +++ b/module/Rest/src/Action/Visit/OrphanVisitsAction.php @@ -7,19 +7,15 @@ namespace Shlinkio\Shlink\Rest\Action\Visit; use Pagerfanta\Pagerfanta; use Psr\Http\Message\ServerRequestInterface; use Shlinkio\Shlink\Core\Visit\Model\OrphanVisitsParams; -use Shlinkio\Shlink\Core\Visit\Model\VisitsParams; use Shlinkio\Shlink\Rest\Entity\ApiKey; class OrphanVisitsAction extends AbstractListVisitsAction { protected const string ROUTE_PATH = '/visits/orphan'; - protected function getVisitsPaginator( - ServerRequestInterface $request, - VisitsParams $params, - ApiKey $apiKey, - ): Pagerfanta { - $orphanParams = OrphanVisitsParams::fromVisitsParamsAndRawData($params, $request->getQueryParams()); + protected function getVisitsPaginator(ServerRequestInterface $request, ApiKey $apiKey): Pagerfanta + { + $orphanParams = OrphanVisitsParams::fromRawData($request->getQueryParams()); return $this->visitsHelper->orphanVisits($orphanParams, $apiKey); } } diff --git a/module/Rest/src/Action/Visit/ShortUrlVisitsAction.php b/module/Rest/src/Action/Visit/ShortUrlVisitsAction.php index d8fc36e9..1d720d35 100644 --- a/module/Rest/src/Action/Visit/ShortUrlVisitsAction.php +++ b/module/Rest/src/Action/Visit/ShortUrlVisitsAction.php @@ -14,8 +14,9 @@ class ShortUrlVisitsAction extends AbstractListVisitsAction { protected const string ROUTE_PATH = '/short-urls/{shortCode}/visits'; - protected function getVisitsPaginator(Request $request, VisitsParams $params, ApiKey $apiKey): Pagerfanta + protected function getVisitsPaginator(Request $request, ApiKey $apiKey): Pagerfanta { + $params = VisitsParams::fromRawData($request->getQueryParams()); $identifier = ShortUrlIdentifier::fromApiRequest($request); return $this->visitsHelper->visitsForShortUrl($identifier, $params, $apiKey); } diff --git a/module/Rest/src/Action/Visit/TagVisitsAction.php b/module/Rest/src/Action/Visit/TagVisitsAction.php index 07ad7167..bf43edaa 100644 --- a/module/Rest/src/Action/Visit/TagVisitsAction.php +++ b/module/Rest/src/Action/Visit/TagVisitsAction.php @@ -6,15 +6,16 @@ namespace Shlinkio\Shlink\Rest\Action\Visit; use Pagerfanta\Pagerfanta; use Psr\Http\Message\ServerRequestInterface as Request; -use Shlinkio\Shlink\Core\Visit\Model\VisitsParams; +use Shlinkio\Shlink\Core\Visit\Model\WithDomainVisitsParams; use Shlinkio\Shlink\Rest\Entity\ApiKey; class TagVisitsAction extends AbstractListVisitsAction { protected const string ROUTE_PATH = '/tags/{tag}/visits'; - protected function getVisitsPaginator(Request $request, VisitsParams $params, ApiKey $apiKey): Pagerfanta + protected function getVisitsPaginator(Request $request, ApiKey $apiKey): Pagerfanta { + $params = WithDomainVisitsParams::fromRawData($request->getQueryParams()); $tag = $request->getAttribute('tag', ''); return $this->visitsHelper->visitsForTag($tag, $params, $apiKey); } diff --git a/module/Rest/src/ApiKey/Repository/ApiKeyRepository.php b/module/Rest/src/ApiKey/Repository/ApiKeyRepository.php index 6b282a07..8a6bc59d 100644 --- a/module/Rest/src/ApiKey/Repository/ApiKeyRepository.php +++ b/module/Rest/src/ApiKey/Repository/ApiKeyRepository.php @@ -5,6 +5,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Rest\ApiKey\Repository; use Doctrine\DBAL\LockMode; +use Doctrine\ORM\QueryBuilder; use Happyr\DoctrineSpecification\Repository\EntitySpecificationRepository; use Shlinkio\Shlink\Rest\ApiKey\Model\ApiKeyMeta; use Shlinkio\Shlink\Rest\Entity\ApiKey; @@ -48,10 +49,9 @@ class ApiKeyRepository extends EntitySpecificationRepository implements ApiKeyRe { $qb = $this->getEntityManager()->createQueryBuilder(); $qb->select('a.id') - ->from(ApiKey::class, 'a') - ->where($qb->expr()->eq('a.name', ':name')) - ->setParameter('name', $name) - ->setMaxResults(1); + ->from(ApiKey::class, 'a'); + + $this->queryBuilderByName($qb, $name); // Lock for update, to avoid a race condition that inserts a duplicate name after we have checked if one existed $query = $qb->getQuery(); @@ -59,4 +59,27 @@ class ApiKeyRepository extends EntitySpecificationRepository implements ApiKeyRe return $query->getOneOrNullResult() !== null; } + + /** + * @inheritDoc + */ + public function deleteByName(string $name): int + { + $qb = $this->getEntityManager()->createQueryBuilder(); + $qb->delete(ApiKey::class, 'a'); + + $this->queryBuilderByName($qb, $name); + + return $qb->getQuery()->execute(); + } + + /** + * Apply a condition by name to a query builder, and ensure only one result is returned + */ + private function queryBuilderByName(QueryBuilder $qb, string $name): void + { + $qb->where($qb->expr()->eq('a.name', ':name')) + ->setParameter('name', $name) + ->setMaxResults(1); + } } diff --git a/module/Rest/src/ApiKey/Repository/ApiKeyRepositoryInterface.php b/module/Rest/src/ApiKey/Repository/ApiKeyRepositoryInterface.php index 32ada38a..3c4d5397 100644 --- a/module/Rest/src/ApiKey/Repository/ApiKeyRepositoryInterface.php +++ b/module/Rest/src/ApiKey/Repository/ApiKeyRepositoryInterface.php @@ -22,4 +22,10 @@ interface ApiKeyRepositoryInterface extends EntityRepositoryInterface, EntitySpe * Checks whether an API key with provided name exists or not */ public function nameExists(string $name): bool; + + /** + * Delete an API key by name + * @return positive-int|0 Number of affected results + */ + public function deleteByName(string $name): int; } diff --git a/module/Rest/src/Middleware/CrossDomainMiddleware.php b/module/Rest/src/Middleware/CrossDomainMiddleware.php index e54c07a7..69801fe9 100644 --- a/module/Rest/src/Middleware/CrossDomainMiddleware.php +++ b/module/Rest/src/Middleware/CrossDomainMiddleware.php @@ -39,12 +39,10 @@ readonly class CrossDomainMiddleware implements MiddlewareInterface, RequestMeth private function addOptionsHeaders(ServerRequestInterface $request, ResponseInterface $response): ResponseInterface { // Options requests should always be empty and have a 204 status code - return EmptyResponse::withHeaders([ - ...$response->getHeaders(), - 'Access-Control-Allow-Methods' => $this->resolveCorsAllowedMethods($response), - 'Access-Control-Allow-Headers' => $request->getHeaderLine('Access-Control-Request-Headers'), - 'Access-Control-Max-Age' => $this->options->maxAge, - ]); + return EmptyResponse::withHeaders($response->getHeaders()) + ->withHeader('Access-Control-Allow-Methods', $this->resolveCorsAllowedMethods($response)) + ->withHeader('Access-Control-Allow-Headers', $request->getHeaderLine('Access-Control-Request-Headers')) + ->withHeader('Access-Control-Max-Age', (string) $this->options->maxAge); } private function resolveCorsAllowedMethods(ResponseInterface $response): string diff --git a/module/Rest/src/Service/ApiKeyService.php b/module/Rest/src/Service/ApiKeyService.php index 5779e86c..a5a85b79 100644 --- a/module/Rest/src/Service/ApiKeyService.php +++ b/module/Rest/src/Service/ApiKeyService.php @@ -68,6 +68,17 @@ readonly class ApiKeyService implements ApiKeyServiceInterface return new ApiKeyCheckResult($apiKey); } + /** + * @inheritDoc + */ + public function deleteByName(string $apiKeyName): void + { + $affectedResults = $this->repo->deleteByName($apiKeyName); + if ($affectedResults === 0) { + throw ApiKeyNotFoundException::forName($apiKeyName); + } + } + /** * @inheritDoc */ diff --git a/module/Rest/src/Service/ApiKeyServiceInterface.php b/module/Rest/src/Service/ApiKeyServiceInterface.php index 745355d7..6eb3df0f 100644 --- a/module/Rest/src/Service/ApiKeyServiceInterface.php +++ b/module/Rest/src/Service/ApiKeyServiceInterface.php @@ -22,6 +22,11 @@ interface ApiKeyServiceInterface public function check(string $key): ApiKeyCheckResult; + /** + * @throws ApiKeyNotFoundException + */ + public function deleteByName(string $apiKeyName): void; + /** * @throws ApiKeyNotFoundException */ diff --git a/module/Rest/test-api/Action/ListShortUrlsTest.php b/module/Rest/test-api/Action/ListShortUrlsTest.php index 60b493d6..14b52d42 100644 --- a/module/Rest/test-api/Action/ListShortUrlsTest.php +++ b/module/Rest/test-api/Action/ListShortUrlsTest.php @@ -153,8 +153,11 @@ class ListShortUrlsTest extends ApiTestCase ]; #[Test, DataProvider('provideFilteredLists')] - public function shortUrlsAreProperlyListed(array $query, array $expectedShortUrls, string $apiKey): void - { + public function shortUrlsAreProperlyListed( + array $query, + array $expectedShortUrls, + string $apiKey = 'valid_api_key', + ): void { $resp = $this->callApiWithKey(self::METHOD_GET, '/short-urls', [RequestOptions::QUERY => $query], $apiKey); $respPayload = $this->getJsonResponsePayload($resp); @@ -176,21 +179,21 @@ class ListShortUrlsTest extends ApiTestCase self::SHORT_URL_CUSTOM_SLUG_AND_DOMAIN, self::SHORT_URL_SHLINK_WITH_TITLE, 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, - ], '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, - ], 'valid_api_key']; + ]]; yield [['orderBy' => 'shortCode'], [ self::SHORT_URL_SHLINK_WITH_TITLE, self::SHORT_URL_CUSTOM_SLUG, @@ -198,7 +201,7 @@ class ListShortUrlsTest extends ApiTestCase self::SHORT_URL_META, self::SHORT_URL_DOCS, self::SHORT_URL_CUSTOM_DOMAIN, - ], 'valid_api_key']; + ]]; yield [['orderBy' => 'shortCode-DESC'], [ self::SHORT_URL_DOCS, self::SHORT_URL_CUSTOM_DOMAIN, @@ -206,7 +209,7 @@ class ListShortUrlsTest extends ApiTestCase self::SHORT_URL_CUSTOM_SLUG_AND_DOMAIN, self::SHORT_URL_CUSTOM_SLUG, self::SHORT_URL_SHLINK_WITH_TITLE, - ], 'valid_api_key']; + ]]; yield [['orderBy' => 'title-DESC'], [ self::SHORT_URL_SHLINK_WITH_TITLE, self::SHORT_URL_META, @@ -214,66 +217,105 @@ class ListShortUrlsTest extends ApiTestCase self::SHORT_URL_DOCS, self::SHORT_URL_CUSTOM_DOMAIN, self::SHORT_URL_CUSTOM_SLUG_AND_DOMAIN, - ], '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, - ], '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, - ], 'valid_api_key']; + ]]; yield [['tags' => ['foo']], [ self::SHORT_URL_CUSTOM_DOMAIN, self::SHORT_URL_META, self::SHORT_URL_SHLINK_WITH_TITLE, - ], 'valid_api_key']; + ]]; yield [['tags' => ['bar']], [ self::SHORT_URL_META, - ], 'valid_api_key']; + ]]; yield [['tags' => ['foo', 'bar']], [ self::SHORT_URL_CUSTOM_DOMAIN, self::SHORT_URL_META, 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, - ], 'valid_api_key']; + ]]; yield [['tags' => ['foo', 'bar'], 'tagsMode' => 'all'], [ self::SHORT_URL_META, - ], 'valid_api_key']; + ]]; yield [['tags' => ['foo', 'bar', 'baz']], [ self::SHORT_URL_CUSTOM_DOMAIN, self::SHORT_URL_META, self::SHORT_URL_SHLINK_WITH_TITLE, - ], 'valid_api_key']; - yield [['tags' => ['foo', 'bar', 'baz'], 'tagsMode' => 'all'], [], 'valid_api_key']; + ]]; + yield [['tags' => ['foo', 'bar', 'baz'], 'tagsMode' => 'all'], []]; yield [['tags' => ['foo'], 'endDate' => Chronos::parse('2018-12-01')->toAtomString()], [ self::SHORT_URL_SHLINK_WITH_TITLE, - ], 'valid_api_key']; + ]]; yield [['searchTerm' => 'alejandro'], [ self::SHORT_URL_CUSTOM_DOMAIN, self::SHORT_URL_META, - ], 'valid_api_key']; + ]]; yield [['searchTerm' => 'cool'], [ self::SHORT_URL_SHLINK_WITH_TITLE, - ], 'valid_api_key']; + ]]; yield [['searchTerm' => 'example.com'], [ self::SHORT_URL_CUSTOM_DOMAIN, - ], 'valid_api_key']; + ]]; yield [['domain' => 'example.com'], [ self::SHORT_URL_CUSTOM_DOMAIN, - ], 'valid_api_key']; + ]]; yield [['domain' => Domain::DEFAULT_AUTHORITY], [ self::SHORT_URL_CUSTOM_SLUG, self::SHORT_URL_META, self::SHORT_URL_SHLINK_WITH_TITLE, self::SHORT_URL_DOCS, - ], 'valid_api_key']; + ]]; + + // Exclude tags + yield [['excludeTags' => ['foo']], [ + self::SHORT_URL_CUSTOM_SLUG, + self::SHORT_URL_CUSTOM_SLUG_AND_DOMAIN, + self::SHORT_URL_DOCS, + ]]; + yield [['excludeTags' => ['foo', 'bar']], [ + self::SHORT_URL_CUSTOM_SLUG, + self::SHORT_URL_CUSTOM_SLUG_AND_DOMAIN, + self::SHORT_URL_DOCS, + ]]; + yield [['excludeTags' => ['bar', 'foo'], 'excludeTagsMode' => 'all'], [ + self::SHORT_URL_CUSTOM_DOMAIN, + self::SHORT_URL_CUSTOM_SLUG, + self::SHORT_URL_CUSTOM_SLUG_AND_DOMAIN, + self::SHORT_URL_SHLINK_WITH_TITLE, + self::SHORT_URL_DOCS, + ]]; + + // Filter by API key name + yield [['apiKeyName' => 'author_api_key'], [ + self::SHORT_URL_CUSTOM_SLUG, + self::SHORT_URL_META, + self::SHORT_URL_SHLINK_WITH_TITLE, + ]]; + yield [['apiKeyName' => 'invalid'], []]; + yield [['apiKeyName' => 'valid_api_key'], [ + // If the author_api_key is used, the `apiKeyName` param is ignored + self::SHORT_URL_CUSTOM_SLUG, + self::SHORT_URL_META, + self::SHORT_URL_SHLINK_WITH_TITLE, + ], 'author_api_key']; + yield [['apiKeyName' => 'valid_api_key'], [ + // If the domain_api_key is used, the `apiKeyName` param is ignored + self::SHORT_URL_CUSTOM_DOMAIN, + ], 'domain_api_key']; + + // Different API keys yield [[], [ self::SHORT_URL_CUSTOM_SLUG, self::SHORT_URL_META, diff --git a/module/Rest/test-api/Action/NonOrphanVisitsTest.php b/module/Rest/test-api/Action/NonOrphanVisitsTest.php index 0e69db54..db38c534 100644 --- a/module/Rest/test-api/Action/NonOrphanVisitsTest.php +++ b/module/Rest/test-api/Action/NonOrphanVisitsTest.php @@ -31,5 +31,7 @@ class NonOrphanVisitsTest extends ApiTestCase yield 'bots excluded' => [['excludeBots' => 'true'], 6, 6]; yield 'bots excluded and pagination' => [['excludeBots' => 'true', 'page' => 1, 'itemsPerPage' => 4], 6, 4]; yield 'date filter' => [['startDate' => Chronos::now()->addDays(1)->toAtomString()], 0, 0]; + yield 'domain filter' => [['domain' => 'example.com'], 0, 0]; + yield 'default domain filter' => [['domain' => 'DEFAULT'], 7, 7]; } } diff --git a/module/Rest/test-api/Action/OrphanVisitsTest.php b/module/Rest/test-api/Action/OrphanVisitsTest.php index f9414899..30519c4c 100644 --- a/module/Rest/test-api/Action/OrphanVisitsTest.php +++ b/module/Rest/test-api/Action/OrphanVisitsTest.php @@ -19,7 +19,7 @@ class OrphanVisitsTest extends ApiTestCase 'userAgent' => 'cf-facebook', 'visitLocation' => null, 'potentialBot' => true, - 'visitedUrl' => 'foo.com', + 'visitedUrl' => 'https://example.com/short', 'type' => 'invalid_short_url', 'redirectUrl' => null, ]; @@ -29,7 +29,7 @@ class OrphanVisitsTest extends ApiTestCase 'userAgent' => 'shlink-tests-agent', 'visitLocation' => null, 'potentialBot' => false, - 'visitedUrl' => '', + 'visitedUrl' => 'https://s.test/bar', 'type' => 'regular_404', 'redirectUrl' => null, ]; @@ -39,7 +39,7 @@ class OrphanVisitsTest extends ApiTestCase 'userAgent' => 'shlink-tests-agent', 'visitLocation' => null, 'potentialBot' => false, - 'visitedUrl' => '', + 'visitedUrl' => 'https://s.test/foo', 'type' => 'base_url', 'redirectUrl' => null, ]; @@ -80,6 +80,14 @@ class OrphanVisitsTest extends ApiTestCase 1, [self::INVALID_SHORT_URL], ]; + yield 'example domain only' => [['domain' => 'example.com'], 1, 1, [self::INVALID_SHORT_URL]]; + yield 'default domain only' => [['domain' => 's.test'], 2, 2, [self::REGULAR_NOT_FOUND, self::BASE_URL]]; + yield 'default domain only with DEFAULT keyword' => [ + ['domain' => 'DEFAULT'], + 2, + 2, + [self::REGULAR_NOT_FOUND, self::BASE_URL], + ]; } #[Test] diff --git a/module/Rest/test-api/Action/TagVisitsTest.php b/module/Rest/test-api/Action/TagVisitsTest.php index c51f02fb..7a0c5f45 100644 --- a/module/Rest/test-api/Action/TagVisitsTest.php +++ b/module/Rest/test-api/Action/TagVisitsTest.php @@ -17,11 +17,11 @@ class TagVisitsTest extends ApiTestCase public function expectedVisitsAreReturned( string $apiKey, string $tag, - bool $excludeBots, + array $query, int $expectedVisitsAmount, ): void { $resp = $this->callApiWithKey(self::METHOD_GET, sprintf('/tags/%s/visits', $tag), [ - RequestOptions::QUERY => $excludeBots ? ['excludeBots' => true] : [], + RequestOptions::QUERY => $query, ], $apiKey); $payload = $this->getJsonResponsePayload($resp); @@ -33,16 +33,18 @@ class TagVisitsTest extends ApiTestCase public static function provideTags(): iterable { - yield 'foo with admin API key' => ['valid_api_key', 'foo', false, 5]; - yield 'foo with admin API key and no bots' => ['valid_api_key', 'foo', true, 4]; - yield 'bar with admin API key' => ['valid_api_key', 'bar', false, 2]; - yield 'bar with admin API key and no bots' => ['valid_api_key', 'bar', true, 1]; - yield 'baz with admin API key' => ['valid_api_key', 'baz', false, 0]; - yield 'foo with author API key' => ['author_api_key', 'foo', false, 5]; - yield 'foo with author API key and no bots' => ['author_api_key', 'foo', true, 4]; - yield 'bar with author API key' => ['author_api_key', 'bar', false, 2]; - yield 'bar with author API key and no bots' => ['author_api_key', 'bar', true, 1]; - yield 'foo with domain API key' => ['domain_api_key', 'foo', false, 0]; + yield 'foo with admin API key' => ['valid_api_key', 'foo', [], 5]; + yield 'foo with admin API key and no bots' => ['valid_api_key', 'foo', ['excludeBots' => true], 4]; + yield 'bar with admin API key' => ['valid_api_key', 'bar', [], 2]; + yield 'bar with admin API key and no bots' => ['valid_api_key', 'bar', ['excludeBots' => true], 1]; + yield 'baz with admin API key' => ['valid_api_key', 'baz', [], 0]; + yield 'foo with author API key' => ['author_api_key', 'foo', [], 5]; + yield 'foo with author API key and no bots' => ['author_api_key', 'foo', ['excludeBots' => true], 4]; + yield 'bar with author API key' => ['author_api_key', 'bar', [], 2]; + yield 'bar with author API key and no bots' => ['author_api_key', 'bar', ['excludeBots' => true], 1]; + yield 'foo with domain API key' => ['domain_api_key', 'foo', [], 0]; + yield 'foo with specific domain' => ['valid_api_key', 'foo', ['domain' => 'example.com'], 0]; + yield 'foo with default domain' => ['valid_api_key', 'foo', ['domain' => 'DEFAULT'], 5]; } #[Test, DataProvider('provideApiKeysAndTags')] diff --git a/module/Rest/test-api/Fixtures/ShortUrlsFixture.php b/module/Rest/test-api/Fixtures/ShortUrlsFixture.php index 31ca6361..aec8098f 100644 --- a/module/Rest/test-api/Fixtures/ShortUrlsFixture.php +++ b/module/Rest/test-api/Fixtures/ShortUrlsFixture.php @@ -98,7 +98,6 @@ class ShortUrlsFixture extends AbstractFixture implements DependentFixtureInterf { $ref = new ReflectionObject($shortUrl); $dateProp = $ref->getProperty('dateCreated'); - $dateProp->setAccessible(true); $dateProp->setValue($shortUrl, Chronos::parse($date)); return $shortUrl; diff --git a/module/Rest/test-api/Fixtures/VisitsFixture.php b/module/Rest/test-api/Fixtures/VisitsFixture.php index e10b6dab..76954ebf 100644 --- a/module/Rest/test-api/Fixtures/VisitsFixture.php +++ b/module/Rest/test-api/Fixtures/VisitsFixture.php @@ -58,20 +58,32 @@ class VisitsFixture extends AbstractFixture implements DependentFixtureInterface Visitor::fromParams('shlink-tests-agent', 'https://app.shlink.io', ''), )); + // Orphan visits (s.test is the default domain in tests env) $manager->persist($this->setVisitDate( - fn () => Visit::forBasePath(Visitor::fromParams('shlink-tests-agent', 'https://s.test', '1.2.3.4')), + fn () => Visit::forBasePath(Visitor::fromParams( + 'shlink-tests-agent', + 'https://s.test', + '1.2.3.4', + visitedUrl: 'https://s.test/foo', + )), '2020-01-01', )); $manager->persist($this->setVisitDate( - fn () => Visit::forRegularNotFound( - Visitor::fromParams('shlink-tests-agent', 'https://s.test/foo/bar', '1.2.3.4'), - ), + fn () => Visit::forRegularNotFound(Visitor::fromParams( + 'shlink-tests-agent', + 'https://s.test/foo/bar', + '1.2.3.4', + visitedUrl: 'https://s.test/bar', + )), '2020-02-01', )); $manager->persist($this->setVisitDate( - fn () => Visit::forInvalidShortUrl( - Visitor::fromParams('cf-facebook', 'https://s.test/foo', '1.2.3.4', 'foo.com'), - ), + fn () => Visit::forInvalidShortUrl(Visitor::fromParams( + 'cf-facebook', + 'https://s.test/foo', + '1.2.3.4', + visitedUrl: 'https://example.com/short', + )), '2020-03-01', )); diff --git a/module/Rest/test-db/ApiKey/Repository/ApiKeyRepositoryTest.php b/module/Rest/test-db/ApiKey/Repository/ApiKeyRepositoryTest.php index d0f6157d..6b3423e9 100644 --- a/module/Rest/test-db/ApiKey/Repository/ApiKeyRepositoryTest.php +++ b/module/Rest/test-db/ApiKey/Repository/ApiKeyRepositoryTest.php @@ -40,4 +40,18 @@ class ApiKeyRepositoryTest extends DatabaseTestCase self::assertTrue($this->repo->nameExists('foo')); self::assertFalse($this->repo->nameExists('bar')); } + + #[Test] + public function deleteByNameReturnsExpectedValue(): void + { + $this->getEntityManager()->persist(ApiKey::fromMeta(ApiKeyMeta::fromParams(name: 'foo'))); + $this->getEntityManager()->flush(); + $this->getEntityManager()->clear(); + + self::assertEquals(0, $this->repo->deleteByName('invalid')); + self::assertEquals(1, $this->repo->deleteByName('foo')); + + // Verify the API key has been deleted + self::assertNull($this->repo->findOneBy(['name' => 'foo'])); + } } diff --git a/module/Rest/test/Middleware/EmptyResponseImplicitOptionsMiddlewareFactoryTest.php b/module/Rest/test/Middleware/EmptyResponseImplicitOptionsMiddlewareFactoryTest.php index 6c051fda..bee602f3 100644 --- a/module/Rest/test/Middleware/EmptyResponseImplicitOptionsMiddlewareFactoryTest.php +++ b/module/Rest/test/Middleware/EmptyResponseImplicitOptionsMiddlewareFactoryTest.php @@ -27,7 +27,6 @@ class EmptyResponseImplicitOptionsMiddlewareFactoryTest extends TestCase $ref = new ReflectionObject($instance); $prop = $ref->getProperty('responseFactory'); - $prop->setAccessible(true); /** @var ResponseFactoryInterface $value */ $value = $prop->getValue($instance); diff --git a/module/Rest/test/Service/ApiKeyServiceTest.php b/module/Rest/test/Service/ApiKeyServiceTest.php index a5b6cecb..146ce0ac 100644 --- a/module/Rest/test/Service/ApiKeyServiceTest.php +++ b/module/Rest/test/Service/ApiKeyServiceTest.php @@ -8,6 +8,7 @@ use Cake\Chronos\Chronos; use Doctrine\ORM\EntityManager; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Test; +use PHPUnit\Framework\Attributes\TestWith; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Shlinkio\Shlink\Common\Exception\InvalidArgumentException; @@ -268,4 +269,19 @@ class ApiKeyServiceTest extends TestCase self::assertSame($apiKey, $result); self::assertEquals('new', $apiKey->name); } + + #[Test] + #[TestWith([0, true])] + #[TestWith([1, false])] + public function deleteByNameThrowsIfNoResultsAreAffected(int $affectedResults, bool $shouldThrow): void + { + $name = 'some_name'; + $this->repo->expects($this->once())->method('deleteByName')->with($name)->willReturn($affectedResults); + + if ($shouldThrow) { + $this->expectException(ApiKeyNotFoundException::class); + } + + $this->service->deleteByName($name); + } }