From 740740b8c60348d9eddb2c61ea4e985fd13d6c4d Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Wed, 12 Apr 2023 19:11:06 +0200 Subject: [PATCH 01/55] Update to latest JamesIves/github-pages-deploy-action --- .github/workflows/publish-swagger-spec.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/publish-swagger-spec.yml b/.github/workflows/publish-swagger-spec.yml index df2fd9c9..9b5dbb32 100644 --- a/.github/workflows/publish-swagger-spec.yml +++ b/.github/workflows/publish-swagger-spec.yml @@ -26,7 +26,7 @@ jobs: - run: mkdir ${{ steps.determine_version.outputs.version }} - run: mv docs/swagger/swagger-inlined.json ${{ steps.determine_version.outputs.version }}/open-api-spec.json - name: Publish spec - uses: JamesIves/github-pages-deploy-action@4.1.7 + uses: JamesIves/github-pages-deploy-action@4 with: token: ${{ secrets.OAS_PUBLISH_TOKEN }} repository-name: 'shlinkio/shlink-open-api-specs' From 06f07e3e400f56d578772568225d264a705b59c8 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Wed, 12 Apr 2023 19:13:35 +0200 Subject: [PATCH 02/55] Update to geekyeggo/delete-artifact@2 --- .github/workflows/ci.yml | 7 ++----- .github/workflows/publish-release.yml | 8 ++------ 2 files changed, 4 insertions(+), 11 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b4335cb2..270c2b0d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -168,10 +168,7 @@ jobs: - upload-coverage runs-on: ubuntu-22.04 steps: - - uses: geekyeggo/delete-artifact@v1 + - uses: geekyeggo/delete-artifact@v2 with: name: | - coverage-unit - coverage-db - coverage-api - coverage-cli + coverage-* diff --git a/.github/workflows/publish-release.yml b/.github/workflows/publish-release.yml index 8d8a4b0d..abfbd456 100644 --- a/.github/workflows/publish-release.yml +++ b/.github/workflows/publish-release.yml @@ -49,11 +49,7 @@ jobs: delete-artifacts: needs: ['publish'] runs-on: ubuntu-22.04 - strategy: - matrix: - php-version: ['8.1', '8.2'] - swoole: ['yes', 'no'] steps: - - uses: geekyeggo/delete-artifact@v1 + - uses: geekyeggo/delete-artifact@v2 with: - name: dist-files-${{ matrix.php-version }}-${{ matrix.swoole }} + name: dist-files-* From 3ba46bbbfa0adb5314e6366c2966b7f61c53b3d5 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 14 Apr 2023 08:58:54 +0200 Subject: [PATCH 03/55] Add support for openswoole 22 --- .github/workflows/ci-db-tests.yml | 2 +- .github/workflows/ci-mutation-tests.yml | 2 +- .github/workflows/ci-tests.yml | 2 +- .github/workflows/ci.yml | 2 +- .github/workflows/publish-release.yml | 2 +- .github/workflows/publish-swagger-spec.yml | 2 +- CHANGELOG.md | 17 +++++++++++++++++ Dockerfile | 2 +- composer.json | 4 ++-- config/container.php | 11 ++++++++++- data/infra/swoole.Dockerfile | 2 +- 11 files changed, 37 insertions(+), 11 deletions(-) diff --git a/.github/workflows/ci-db-tests.yml b/.github/workflows/ci-db-tests.yml index 6bf9ad2c..0c77ded6 100644 --- a/.github/workflows/ci-db-tests.yml +++ b/.github/workflows/ci-db-tests.yml @@ -27,7 +27,7 @@ jobs: - uses: './.github/actions/ci-setup' with: php-version: ${{ matrix.php-version }} - php-extensions: openswoole-4.12.1, pdo_sqlsrv-5.10.1 + php-extensions: openswoole-22.0.0, pdo_sqlsrv-5.10.1 extensions-cache-key: db-tests-extensions-${{ matrix.php-version }}-${{ inputs.platform }} - name: Create test database if: ${{ inputs.platform == 'ms' }} diff --git a/.github/workflows/ci-mutation-tests.yml b/.github/workflows/ci-mutation-tests.yml index 237b37cb..20e5aefa 100644 --- a/.github/workflows/ci-mutation-tests.yml +++ b/.github/workflows/ci-mutation-tests.yml @@ -19,7 +19,7 @@ jobs: - uses: './.github/actions/ci-setup' with: php-version: ${{ matrix.php-version }} - php-extensions: openswoole-4.12.1 + php-extensions: openswoole-22.0.0 extensions-cache-key: mutation-tests-extensions-${{ matrix.php-version }}-${{ inputs.test-group }} - uses: actions/download-artifact@v3 with: diff --git a/.github/workflows/ci-tests.yml b/.github/workflows/ci-tests.yml index ba9fb991..043a3824 100644 --- a/.github/workflows/ci-tests.yml +++ b/.github/workflows/ci-tests.yml @@ -25,7 +25,7 @@ jobs: - uses: './.github/actions/ci-setup' with: php-version: ${{ matrix.php-version }} - php-extensions: openswoole-4.12.1 + php-extensions: openswoole-22.0.0 extensions-cache-key: tests-extensions-${{ matrix.php-version }}-${{ inputs.test-group }} - run: composer test:${{ inputs.test-group }}:ci - uses: actions/upload-artifact@v3 diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 270c2b0d..f01aa66f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -36,7 +36,7 @@ jobs: - uses: './.github/actions/ci-setup' with: php-version: ${{ matrix.php-version }} - php-extensions: openswoole-4.12.1 + php-extensions: openswoole-22.0.0 extensions-cache-key: tests-extensions-${{ matrix.php-version }}-${{ matrix.command }} - run: composer ${{ matrix.command }} diff --git a/.github/workflows/publish-release.yml b/.github/workflows/publish-release.yml index abfbd456..bda463a9 100644 --- a/.github/workflows/publish-release.yml +++ b/.github/workflows/publish-release.yml @@ -17,7 +17,7 @@ jobs: - uses: './.github/actions/ci-setup' with: php-version: ${{ matrix.php-version }} - php-extensions: openswoole-4.12.1 + php-extensions: openswoole-22.0.0 extensions-cache-key: publish-swagger-spec-extensions-${{ matrix.php-version }} install-deps: 'no' - if: ${{ matrix.swoole == 'yes' }} diff --git a/.github/workflows/publish-swagger-spec.yml b/.github/workflows/publish-swagger-spec.yml index 9b5dbb32..f7be6502 100644 --- a/.github/workflows/publish-swagger-spec.yml +++ b/.github/workflows/publish-swagger-spec.yml @@ -20,7 +20,7 @@ jobs: - uses: './.github/actions/ci-setup' with: php-version: ${{ matrix.php-version }} - php-extensions: openswoole-4.12.1 + php-extensions: openswoole-22.0.0 extensions-cache-key: publish-swagger-spec-extensions-${{ matrix.php-version }} - run: composer swagger:inline - run: mkdir ${{ steps.determine_version.outputs.version }} diff --git a/CHANGELOG.md b/CHANGELOG.md index 51f1facc..b51d9c8b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,23 @@ 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). +## [Unreleased] +### Added +* [#1656](https://github.com/shlinkio/shlink/issues/1656) Add support for openswoole 22 + +### Changed +* *Nothing* + +### Deprecated +* *Nothing* + +### Removed +* *Nothing* + +### Fixed +* *Nothing* + + ## [3.5.4] - 2023-04-12 ### Added * *Nothing* diff --git a/Dockerfile b/Dockerfile index 935c3d44..5116c57e 100644 --- a/Dockerfile +++ b/Dockerfile @@ -4,7 +4,7 @@ ARG SHLINK_VERSION=latest ENV SHLINK_VERSION ${SHLINK_VERSION} ARG SHLINK_RUNTIME=openswoole ENV SHLINK_RUNTIME ${SHLINK_RUNTIME} -ENV OPENSWOOLE_VERSION 4.12.1 +ENV OPENSWOOLE_VERSION 22.0.0 ENV PDO_SQLSRV_VERSION 5.10.1 ENV MS_ODBC_DOWNLOAD 'b/9/f/b9f3cce4-3925-46d4-9f46-da08869c6486' ENV MS_ODBC_SQL_VERSION 18_18.1.1.1 diff --git a/composer.json b/composer.json index 03ff1457..48e949c1 100644 --- a/composer.json +++ b/composer.json @@ -63,7 +63,7 @@ "cebe/php-openapi": "^1.7", "devster/ubench": "^2.1", "infection/infection": "^0.26.19", - "openswoole/ide-helper": "~4.11.5", + "openswoole/ide-helper": "~22.0.0", "phpstan/phpstan": "^1.9", "phpstan/phpstan-doctrine": "^1.3", "phpstan/phpstan-phpunit": "^1.3", @@ -107,7 +107,7 @@ "@parallel cs stan swagger:validate test:unit:ci test:db:sqlite:ci test:db:mysql test:db:maria test:db:postgres test:db:ms", "@parallel infect:test:api infect:test:cli infect:ci:unit infect:ci:db" ], - "cs": "phpcs", + "cs": "phpcs -s", "cs:fix": "phpcbf", "stan": "APP_ENV=test php vendor/bin/phpstan analyse module/*/src module/*/test* module/*/config config docker/config data/migrations --level=8", "test": [ diff --git a/config/container.php b/config/container.php index 6e95e84d..6813ebd4 100644 --- a/config/container.php +++ b/config/container.php @@ -12,6 +12,16 @@ chdir(dirname(__DIR__)); require 'vendor/autoload.php'; +// Workaround to make this compatible with both openswoole 22 and earlier versions. +if (! function_exists('swoole_set_process_name')) { + // phpcs:disable + function swoole_set_process_name(string $name): void + { + OpenSwoole\Util::setProcessName($name); + } + // phpcs:enable +} + // This is one of the first files loaded. Configure the timezone here date_default_timezone_set(EnvVars::TIMEZONE->loadFromEnv(date_default_timezone_get())); @@ -21,7 +31,6 @@ if (! class_exists(LOCAL_LOCK_FACTORY)) { class_alias(Lock\LockFactory::class, LOCAL_LOCK_FACTORY); } -// Build container return (static function (): ServiceManager { $config = require __DIR__ . '/config.php'; $container = new ServiceManager($config['dependencies']); diff --git a/data/infra/swoole.Dockerfile b/data/infra/swoole.Dockerfile index 6cab2561..42c27b14 100644 --- a/data/infra/swoole.Dockerfile +++ b/data/infra/swoole.Dockerfile @@ -3,7 +3,7 @@ MAINTAINER Alejandro Celaya ENV APCU_VERSION 5.1.21 ENV INOTIFY_VERSION 3.0.0 -ENV OPENSWOOLE_VERSION 4.12.1 +ENV OPENSWOOLE_VERSION 22.0.0 ENV PDO_SQLSRV_VERSION 5.10.1 ENV MS_ODBC_DOWNLOAD 'b/9/f/b9f3cce4-3925-46d4-9f46-da08869c6486' ENV MS_ODBC_SQL_VERSION 18_18.1.1.1 From b45d8de27d952c4ce08d7dee9c9cae6110c91959 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 14 Apr 2023 09:02:17 +0200 Subject: [PATCH 04/55] Ignore openswoole dep on roadrunner tests CI --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f01aa66f..15baf63e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -69,7 +69,7 @@ jobs: with: php-version: ${{ matrix.php-version }} tools: composer - - run: composer install --no-interaction --prefer-dist + - run: composer install --no-interaction --prefer-dist --ignore-platform-req=ext-openswoole - run: ./vendor/bin/rr get --no-interaction --location bin/ && chmod +x bin/rr - run: composer test:api:rr From f2196583c82a4ec769e3c31027dd885b2a535dff Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 14 Apr 2023 09:44:01 +0200 Subject: [PATCH 05/55] Update phpunit configs to fulfil v10.1 --- composer.json | 2 +- phpunit-api.xml | 8 ++++---- phpunit-cli.xml | 8 ++++---- phpunit-db.xml | 16 ++++++++-------- phpunit.xml.dist | 18 +++++++++--------- 5 files changed, 26 insertions(+), 26 deletions(-) diff --git a/composer.json b/composer.json index 48e949c1..0c43fdb6 100644 --- a/composer.json +++ b/composer.json @@ -69,7 +69,7 @@ "phpstan/phpstan-phpunit": "^1.3", "phpstan/phpstan-symfony": "^1.2", "phpunit/php-code-coverage": "^10.0", - "phpunit/phpunit": "^10.0", + "phpunit/phpunit": "^10.1", "roave/security-advisories": "dev-master", "shlinkio/php-coding-standard": "~2.3.0", "shlinkio/shlink-test-utils": "^3.5", diff --git a/phpunit-api.xml b/phpunit-api.xml index 62ba2179..a2f4def8 100644 --- a/phpunit-api.xml +++ b/phpunit-api.xml @@ -12,10 +12,10 @@ - + - ./module/Core/src - ./module/Rest/src + ./module/Core/src + ./module/Rest/src - + diff --git a/phpunit-cli.xml b/phpunit-cli.xml index 1a129b78..1eaa0f28 100644 --- a/phpunit-cli.xml +++ b/phpunit-cli.xml @@ -12,10 +12,10 @@ - + - ./module/CLI/src - ./module/Core/src + ./module/CLI/src + ./module/Core/src - + diff --git a/phpunit-db.xml b/phpunit-db.xml index 0d2f4dd8..b883d8ca 100644 --- a/phpunit-db.xml +++ b/phpunit-db.xml @@ -12,14 +12,14 @@ - + - ./module/*/src/Repository - ./module/*/src/**/Repository - ./module/*/src/**/**/Repository - ./module/*/src/Spec - ./module/*/src/**/Spec - ./module/*/src/**/**/Spec + ./module/*/src/Repository + ./module/*/src/**/Repository + ./module/*/src/**/**/Repository + ./module/*/src/Spec + ./module/*/src/**/Spec + ./module/*/src/**/**/Spec - + diff --git a/phpunit.xml.dist b/phpunit.xml.dist index 29116d0d..5abec3eb 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -18,17 +18,17 @@ - + - ./module/*/src + ./module/*/src - ./module/Core/src/Repository - ./module/Core/src/**/Repository - ./module/Core/src/**/**/Repository - ./module/Core/src/Spec - ./module/Core/src/**/Spec - ./module/Core/src/**/**/Spec + ./module/Core/src/Repository + ./module/Core/src/**/Repository + ./module/Core/src/**/**/Repository + ./module/Core/src/Spec + ./module/Core/src/**/Spec + ./module/Core/src/**/**/Spec - + From 4ee9c9bbe36c9995790586c276b4c8ca21b3eeb6 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 18 Apr 2023 23:04:58 +0200 Subject: [PATCH 06/55] Migrate to shlinkio/shlink-json --- composer.json | 7 ++++--- module/Rest/src/Middleware/BodyParserMiddleware.php | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/composer.json b/composer.json index 0c43fdb6..d5ac6410 100644 --- a/composer.json +++ b/composer.json @@ -45,12 +45,13 @@ "php-middleware/request-id": "^4.1", "pugx/shortid-php": "^1.1", "ramsey/uuid": "^4.7", - "shlinkio/shlink-common": "^5.4", + "shlinkio/shlink-common": "dev-main#9eecf8c as 5.5", "shlinkio/shlink-config": "^2.4", "shlinkio/shlink-event-dispatcher": "^2.6", - "shlinkio/shlink-importer": "^5.0", + "shlinkio/shlink-importer": "dev-main#6b63b12 as 5.1", "shlinkio/shlink-installer": "^8.3", "shlinkio/shlink-ip-geolocation": "^3.2", + "shlinkio/shlink-json": "^1.0", "spiral/roadrunner": "^2.12", "spiral/roadrunner-jobs": "^2.7", "symfony/console": "^6.2", @@ -72,7 +73,7 @@ "phpunit/phpunit": "^10.1", "roave/security-advisories": "dev-master", "shlinkio/php-coding-standard": "~2.3.0", - "shlinkio/shlink-test-utils": "^3.5", + "shlinkio/shlink-test-utils": "^3.6", "symfony/var-dumper": "^6.2", "veewee/composer-run-parallel": "^1.2" }, diff --git a/module/Rest/src/Middleware/BodyParserMiddleware.php b/module/Rest/src/Middleware/BodyParserMiddleware.php index 68fc1b38..c31bc268 100644 --- a/module/Rest/src/Middleware/BodyParserMiddleware.php +++ b/module/Rest/src/Middleware/BodyParserMiddleware.php @@ -13,7 +13,7 @@ use Psr\Http\Server\RequestHandlerInterface; use Shlinkio\Shlink\Core\Exception\MalformedBodyException; use function Functional\contains; -use function Shlinkio\Shlink\Common\json_decode; +use function Shlinkio\Shlink\Json\json_decode; class BodyParserMiddleware implements MiddlewareInterface, RequestMethodInterface { From a671d555cb3fd1d8a55352a2ba08e3cfb17e6550 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 18 Apr 2023 23:22:48 +0200 Subject: [PATCH 07/55] Update to roadrunner 2023 --- Dockerfile | 2 +- build.sh | 2 +- composer.json | 8 +++++--- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/Dockerfile b/Dockerfile index 5116c57e..c52ec93a 100644 --- a/Dockerfile +++ b/Dockerfile @@ -45,7 +45,7 @@ COPY --from=composer:2 /usr/bin/composer ./composer.phar RUN apk add --no-cache git && \ php composer.phar install --no-dev --prefer-dist --optimize-autoloader --no-progress --no-interaction && \ if [ "$SHLINK_RUNTIME" == 'openswoole' ]; then \ - php composer.phar remove spiral/roadrunner spiral/roadrunner-jobs --with-all-dependencies --update-no-dev --optimize-autoloader --no-progress --no-interactionc ; \ + php composer.phar remove spiral/roadrunner spiral/roadrunner-jobs spiral/roadrunner-cli spiral/roadrunner-http --with-all-dependencies --update-no-dev --optimize-autoloader --no-progress --no-interactionc ; \ elif [ $SHLINK_RUNTIME == 'rr' ]; then \ php composer.phar remove mezzio/mezzio-swoole --with-all-dependencies --update-no-dev --optimize-autoloader --no-progress --no-interaction ; \ fi; \ diff --git a/build.sh b/build.sh index d9cda64d..43b240a2 100755 --- a/build.sh +++ b/build.sh @@ -39,7 +39,7 @@ if [[ $noSwoole ]]; then ${composerBin} remove mezzio/mezzio-swoole --with-all-dependencies --update-no-dev $composerFlags else # If generating a dist for openswoole, uninstall RoadRunner - ${composerBin} remove spiral/roadrunner spiral/roadrunner-jobs --with-all-dependencies --update-no-dev $composerFlags + ${composerBin} remove spiral/roadrunner spiral/roadrunner-jobs spiral/roadrunner-cli spiral/roadrunner-http --with-all-dependencies --update-no-dev $composerFlags fi # Delete development files diff --git a/composer.json b/composer.json index d5ac6410..826f4896 100644 --- a/composer.json +++ b/composer.json @@ -47,13 +47,15 @@ "ramsey/uuid": "^4.7", "shlinkio/shlink-common": "dev-main#9eecf8c as 5.5", "shlinkio/shlink-config": "^2.4", - "shlinkio/shlink-event-dispatcher": "^2.6", + "shlinkio/shlink-event-dispatcher": "dev-main#8c677ae as 3.0", "shlinkio/shlink-importer": "dev-main#6b63b12 as 5.1", "shlinkio/shlink-installer": "^8.3", "shlinkio/shlink-ip-geolocation": "^3.2", "shlinkio/shlink-json": "^1.0", - "spiral/roadrunner": "^2.12", - "spiral/roadrunner-jobs": "^2.7", + "spiral/roadrunner": "^2023.1", + "spiral/roadrunner-cli": "^2.4", + "spiral/roadrunner-http": "^3.0", + "spiral/roadrunner-jobs": "^4.0", "symfony/console": "^6.2", "symfony/filesystem": "^6.2", "symfony/lock": "^6.2", From e652166289553944a912b627c0a261bb9de2ff4c Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 18 Apr 2023 23:24:21 +0200 Subject: [PATCH 08/55] Update changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b51d9c8b..f9ca10a0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this * [#1656](https://github.com/shlinkio/shlink/issues/1656) Add support for openswoole 22 ### Changed -* *Nothing* +* [#1755](https://github.com/shlinkio/shlink/issues/1755) Update to roadrunner 2023 ### Deprecated * *Nothing* From 73150471e9cb3db66399e2308d8919ad7abcdc55 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Wed, 19 Apr 2023 18:57:35 +0200 Subject: [PATCH 09/55] Fix docker image build --- Dockerfile | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/Dockerfile b/Dockerfile index c52ec93a..0dd2d8ff 100644 --- a/Dockerfile +++ b/Dockerfile @@ -43,10 +43,11 @@ FROM base as builder COPY . . COPY --from=composer:2 /usr/bin/composer ./composer.phar RUN apk add --no-cache git && \ - php composer.phar install --no-dev --prefer-dist --optimize-autoloader --no-progress --no-interaction && \ + # FIXME Ignoring ext-openswoole platform req, as it makes install fail with roadrunner, even though it's a dev dependency and we are passing --no-dev + php composer.phar install --no-dev --prefer-dist --optimize-autoloader --no-progress --no-interaction --ignore-platform-req=ext-openswoole && \ if [ "$SHLINK_RUNTIME" == 'openswoole' ]; then \ - php composer.phar remove spiral/roadrunner spiral/roadrunner-jobs spiral/roadrunner-cli spiral/roadrunner-http --with-all-dependencies --update-no-dev --optimize-autoloader --no-progress --no-interactionc ; \ - elif [ $SHLINK_RUNTIME == 'rr' ]; then \ + php composer.phar remove spiral/roadrunner spiral/roadrunner-jobs spiral/roadrunner-cli spiral/roadrunner-http --with-all-dependencies --update-no-dev --optimize-autoloader --no-progress --no-interaction ; \ + elif [ "$SHLINK_RUNTIME" == 'rr' ]; then \ php composer.phar remove mezzio/mezzio-swoole --with-all-dependencies --update-no-dev --optimize-autoloader --no-progress --no-interaction ; \ fi; \ php composer.phar clear-cache && \ From c582eba753196723595e3feb3474d49ea5178a67 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 22 Apr 2023 19:44:04 +0200 Subject: [PATCH 10/55] Make sure short URL domain is resolved as null when default one is provided --- .../Command/ShortUrl/CreateShortUrlCommand.php | 9 --------- .../ShortUrl/CreateShortUrlCommandTest.php | 9 +++------ module/Core/config/dependencies.config.php | 2 +- module/Core/src/Options/UrlShortenerOptions.php | 5 +++++ .../PersistenceShortUrlRelationResolver.php | 15 ++++++++------- .../Core/src/ShortUrl/ShortUrlListService.php | 2 +- .../PersistenceShortUrlRelationResolverTest.php | 17 +++++++++++++---- 7 files changed, 31 insertions(+), 28 deletions(-) diff --git a/module/CLI/src/Command/ShortUrl/CreateShortUrlCommand.php b/module/CLI/src/Command/ShortUrl/CreateShortUrlCommand.php index bb332d82..a998a677 100644 --- a/module/CLI/src/Command/ShortUrl/CreateShortUrlCommand.php +++ b/module/CLI/src/Command/ShortUrl/CreateShortUrlCommand.php @@ -31,7 +31,6 @@ class CreateShortUrlCommand extends Command public const NAME = 'short-url:create'; private ?SymfonyStyle $io; - private string $defaultDomain; public function __construct( private readonly UrlShortenerInterface $urlShortener, @@ -39,7 +38,6 @@ class CreateShortUrlCommand extends Command private readonly UrlShortenerOptions $options, ) { parent::__construct(); - $this->defaultDomain = $this->options->domain['hostname'] ?? ''; } protected function configure(): void @@ -121,7 +119,6 @@ class CreateShortUrlCommand extends Command protected function interact(InputInterface $input, OutputInterface $output): void { $this->verifyLongUrlArgument($input, $output); - $this->verifyDomainArgument($input); } private function verifyLongUrlArgument(InputInterface $input, OutputInterface $output): void @@ -138,12 +135,6 @@ class CreateShortUrlCommand extends Command } } - private function verifyDomainArgument(InputInterface $input): void - { - $domain = $input->getOption('domain'); - $input->setOption('domain', $domain === $this->defaultDomain ? null : $domain); - } - protected function execute(InputInterface $input, OutputInterface $output): ?int { $io = $this->getIO($input, $output); diff --git a/module/CLI/test/Command/ShortUrl/CreateShortUrlCommandTest.php b/module/CLI/test/Command/ShortUrl/CreateShortUrlCommandTest.php index fd474007..60482138 100644 --- a/module/CLI/test/Command/ShortUrl/CreateShortUrlCommandTest.php +++ b/module/CLI/test/Command/ShortUrl/CreateShortUrlCommandTest.php @@ -28,8 +28,6 @@ class CreateShortUrlCommandTest extends TestCase { use CliTestUtilsTrait; - private const DEFAULT_DOMAIN = 'default.com'; - private CommandTester $commandTester; private MockObject & UrlShortenerInterface $urlShortener; private MockObject & ShortUrlStringifierInterface $stringifier; @@ -43,7 +41,7 @@ class CreateShortUrlCommandTest extends TestCase $this->urlShortener, $this->stringifier, new UrlShortenerOptions( - domain: ['hostname' => self::DEFAULT_DOMAIN, 'schema' => ''], + domain: ['hostname' => 'example.com', 'schema' => ''], defaultShortCodesLength: 5, ), ); @@ -147,9 +145,8 @@ class CreateShortUrlCommandTest extends TestCase public static function provideDomains(): iterable { yield 'no domain' => [[], null]; - yield 'non-default domain foo' => [['--domain' => 'foo.com'], 'foo.com']; - yield 'non-default domain bar' => [['-d' => 'bar.com'], 'bar.com']; - yield 'default domain' => [['--domain' => self::DEFAULT_DOMAIN], null]; + yield 'domain foo' => [['--domain' => 'foo.com'], 'foo.com']; + yield 'domain bar' => [['-d' => 'bar.com'], 'bar.com']; } #[Test, DataProvider('provideFlags')] diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index 567d57ce..008db777 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -161,7 +161,7 @@ return [ ], Action\RobotsAction::class => [Crawling\CrawlingHelper::class], - ShortUrl\Resolver\PersistenceShortUrlRelationResolver::class => ['em'], + ShortUrl\Resolver\PersistenceShortUrlRelationResolver::class => ['em', Options\UrlShortenerOptions::class], ShortUrl\Helper\ShortUrlStringifier::class => ['config.url_shortener.domain', 'config.router.base_path'], ShortUrl\Helper\ShortUrlTitleResolutionHelper::class => [Util\UrlValidator::class], ShortUrl\Helper\ShortUrlRedirectionBuilder::class => [Options\TrackingOptions::class], diff --git a/module/Core/src/Options/UrlShortenerOptions.php b/module/Core/src/Options/UrlShortenerOptions.php index 6e6ac087..32b40033 100644 --- a/module/Core/src/Options/UrlShortenerOptions.php +++ b/module/Core/src/Options/UrlShortenerOptions.php @@ -26,4 +26,9 @@ final class UrlShortenerOptions { return $this->mode === ShortUrlMode::LOOSE; } + + public function defaultDomain(): string + { + return $this->domain['hostname'] ?? ''; + } } diff --git a/module/Core/src/ShortUrl/Resolver/PersistenceShortUrlRelationResolver.php b/module/Core/src/ShortUrl/Resolver/PersistenceShortUrlRelationResolver.php index db6721d5..48ec634e 100644 --- a/module/Core/src/ShortUrl/Resolver/PersistenceShortUrlRelationResolver.php +++ b/module/Core/src/ShortUrl/Resolver/PersistenceShortUrlRelationResolver.php @@ -9,6 +9,7 @@ use Doctrine\Common\Collections\Collection; use Doctrine\ORM\EntityManagerInterface; use Doctrine\ORM\Events; use Shlinkio\Shlink\Core\Domain\Entity\Domain; +use Shlinkio\Shlink\Core\Options\UrlShortenerOptions; use Shlinkio\Shlink\Core\Tag\Entity\Tag; use function Functional\map; @@ -21,15 +22,17 @@ class PersistenceShortUrlRelationResolver implements ShortUrlRelationResolverInt /** @var array */ private array $memoizedNewTags = []; - public function __construct(private readonly EntityManagerInterface $em) - { + public function __construct( + private readonly EntityManagerInterface $em, + private readonly UrlShortenerOptions $options = new UrlShortenerOptions(), + ) { // Registering this as an event listener will make the postFlush method to be called automatically $this->em->getEventManager()->addEventListener(Events::postFlush, $this); } public function resolveDomain(?string $domain): ?Domain { - if ($domain === null) { + if ($domain === null || $domain === $this->options->defaultDomain()) { return null; } @@ -42,9 +45,7 @@ class PersistenceShortUrlRelationResolver implements ShortUrlRelationResolverInt private function memoizeNewDomain(string $domain): Domain { - return $this->memoizedNewDomains[$domain] = $this->memoizedNewDomains[$domain] ?? Domain::withAuthority( - $domain, - ); + return $this->memoizedNewDomains[$domain] ??= Domain::withAuthority($domain); } /** @@ -71,7 +72,7 @@ class PersistenceShortUrlRelationResolver implements ShortUrlRelationResolverInt private function memoizeNewTag(string $tagName): Tag { - return $this->memoizedNewTags[$tagName] = $this->memoizedNewTags[$tagName] ?? new Tag($tagName); + return $this->memoizedNewTags[$tagName] ??= new Tag($tagName); } public function postFlush(): void diff --git a/module/Core/src/ShortUrl/ShortUrlListService.php b/module/Core/src/ShortUrl/ShortUrlListService.php index d83647f0..60f56554 100644 --- a/module/Core/src/ShortUrl/ShortUrlListService.php +++ b/module/Core/src/ShortUrl/ShortUrlListService.php @@ -25,7 +25,7 @@ class ShortUrlListService implements ShortUrlListServiceInterface */ public function listShortUrls(ShortUrlsParams $params, ?ApiKey $apiKey = null): Paginator { - $defaultDomain = $this->urlShortenerOptions->domain['hostname'] ?? ''; + $defaultDomain = $this->urlShortenerOptions->defaultDomain(); $paginator = new Paginator(new ShortUrlRepositoryAdapter($this->repo, $params, $apiKey, $defaultDomain)); $paginator->setMaxPerPage($params->itemsPerPage) ->setCurrentPage($params->page); diff --git a/module/Core/test/ShortUrl/Resolver/PersistenceShortUrlRelationResolverTest.php b/module/Core/test/ShortUrl/Resolver/PersistenceShortUrlRelationResolverTest.php index 43f99462..d31f3d9b 100644 --- a/module/Core/test/ShortUrl/Resolver/PersistenceShortUrlRelationResolverTest.php +++ b/module/Core/test/ShortUrl/Resolver/PersistenceShortUrlRelationResolverTest.php @@ -12,6 +12,7 @@ use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Shlinkio\Shlink\Core\Domain\Entity\Domain; use Shlinkio\Shlink\Core\Domain\Repository\DomainRepositoryInterface; +use Shlinkio\Shlink\Core\Options\UrlShortenerOptions; use Shlinkio\Shlink\Core\ShortUrl\Resolver\PersistenceShortUrlRelationResolver; use Shlinkio\Shlink\Core\Tag\Entity\Tag; use Shlinkio\Shlink\Core\Tag\Repository\TagRepositoryInterface; @@ -28,14 +29,22 @@ class PersistenceShortUrlRelationResolverTest extends TestCase $this->em = $this->createMock(EntityManagerInterface::class); $this->em->method('getEventManager')->willReturn(new EventManager()); - $this->resolver = new PersistenceShortUrlRelationResolver($this->em); + $this->resolver = new PersistenceShortUrlRelationResolver($this->em, new UrlShortenerOptions( + domain: ['schema' => 'https', 'hostname' => 'default.com'], + )); } - #[Test] - public function returnsEmptyWhenNoDomainIsProvided(): void + #[Test, DataProvider('provideDomainsThatEmpty')] + public function returnsEmptyInSomeCases(?string $domain): void { $this->em->expects($this->never())->method('getRepository')->with(Domain::class); - self::assertNull($this->resolver->resolveDomain(null)); + self::assertNull($this->resolver->resolveDomain($domain)); + } + + public static function provideDomainsThatEmpty(): iterable + { + yield 'null' => [null]; + yield 'default domain' => ['default.com']; } #[Test, DataProvider('provideFoundDomains')] From f2ecbceae9f44541fefaba25b063124e31c851ba Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 22 Apr 2023 19:46:28 +0200 Subject: [PATCH 11/55] Update changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f9ca10a0..f9c1dc57 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,7 +18,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this * *Nothing* ### Fixed -* *Nothing* +* [#1760](https://github.com/shlinkio/shlink/issues/1760) Fix domain not being set to null when importing short URLs with default domain. ## [3.5.4] - 2023-04-12 From cf49393ef2d183ad9b74d0d63176a0d34e5ca911 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 23 Apr 2023 11:19:05 +0200 Subject: [PATCH 12/55] Add --show-domain flag to list short URLs command --- .../Command/ShortUrl/ListShortUrlsCommand.php | 10 +++++++ .../ShortUrl/ListShortUrlsCommandTest.php | 29 ++++++++++++++++--- 2 files changed, 35 insertions(+), 4 deletions(-) diff --git a/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php b/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php index 7a9c77af..9202957b 100644 --- a/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php +++ b/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php @@ -102,6 +102,12 @@ class ListShortUrlsCommand extends Command InputOption::VALUE_NONE, 'Whether to display the tags or not.', ) + ->addOption( + 'show-domain', + null, + InputOption::VALUE_NONE, + 'Whether to display the domain or not. Those belonging to default domain will have value "DEFAULT".', + ) ->addOption( 'show-api-key', 'k', @@ -217,6 +223,10 @@ class ListShortUrlsCommand extends Command if ($input->getOption('show-tags')) { $columnsMap['Tags'] = static fn (array $shortUrl): string => implode(', ', $shortUrl['tags']); } + if ($input->getOption('show-domain')) { + $columnsMap['Domain'] = static fn (array $_, ShortUrl $shortUrl): string => + $shortUrl->getDomain()?->authority ?? 'DEFAULT'; + } if ($input->getOption('show-api-key')) { $columnsMap['API Key'] = static fn (array $_, ShortUrl $shortUrl): string => $shortUrl->authorApiKey()?->__toString() ?? ''; diff --git a/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php b/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php index 52d1eeb3..d81172ed 100644 --- a/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php +++ b/module/CLI/test/Command/ShortUrl/ListShortUrlsCommandTest.php @@ -144,13 +144,19 @@ class ListShortUrlsCommandTest extends TestCase yield 'tags only' => [ ['--show-tags' => true], ['| Tags ', '| foo, bar, baz'], - ['| API Key ', '| API Key Name |', $key, '| my api key'], + ['| API Key ', '| API Key Name |', $key, '| my api key', '| Domain', '| DEFAULT'], + $apiKey, + ]; + yield 'domain only' => [ + ['--show-domain' => true], + ['| Domain', '| DEFAULT'], + ['| Tags ', '| foo, bar, baz', '| API Key ', '| API Key Name |', $key, '| my api key'], $apiKey, ]; yield 'api key only' => [ ['--show-api-key' => true], ['| API Key ', $key], - ['| Tags ', '| foo, bar, baz', '| API Key Name |', '| my api key'], + ['| Tags ', '| foo, bar, baz', '| API Key Name |', '| my api key', '| Domain', '| DEFAULT'], $apiKey, ]; yield 'api key name only' => [ @@ -165,9 +171,24 @@ class ListShortUrlsCommandTest extends TestCase ['| API Key Name |', '| my api key'], $apiKey, ]; + yield 'tags and domain' => [ + ['--show-tags' => true, '--show-domain' => true], + ['| Tags ', '| foo, bar, baz', '| Domain', '| DEFAULT'], + ['| API Key Name |', '| my api key'], + $apiKey, + ]; yield 'all' => [ - ['--show-tags' => true, '--show-api-key' => true, '--show-api-key-name' => true], - ['| API Key ', '| Tags ', '| API Key Name |', '| foo, bar, baz', $key, '| my api key'], + ['--show-tags' => true, '--show-domain' => true, '--show-api-key' => true, '--show-api-key-name' => true], + [ + '| API Key ', + '| Tags ', + '| API Key Name |', + '| foo, bar, baz', + $key, + '| my api key', + '| Domain', + '| DEFAULT', + ], [], $apiKey, ]; From 1b8334499598d20383abb0f523b6ea081ad7bdaa Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 23 Apr 2023 11:20:54 +0200 Subject: [PATCH 13/55] Create CLI test checking default domain is ignored even if explicitly provided --- .../test-cli/Command/CreateShortUrlTest.php | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 module/CLI/test-cli/Command/CreateShortUrlTest.php diff --git a/module/CLI/test-cli/Command/CreateShortUrlTest.php b/module/CLI/test-cli/Command/CreateShortUrlTest.php new file mode 100644 index 00000000..d4d8a583 --- /dev/null +++ b/module/CLI/test-cli/Command/CreateShortUrlTest.php @@ -0,0 +1,31 @@ +exec( + [CreateShortUrlCommand::NAME, 'https://example.com', '--domain', $defaultDomain, '--custom-slug', $slug], + ); + + self::assertEquals(ExitCodes::EXIT_SUCCESS, $exitCode); + self::assertStringContainsString('Generated short URL: http://' . $defaultDomain . '/' . $slug, $output); + + [$listOutput] = $this->exec([ListShortUrlsCommand::NAME, '--show-domain', '--search-term', $slug]); + self::assertStringContainsString('DEFAULT', $listOutput); + } +} From d06e92ffc269617c9ccca2c19865cbf54de27a71 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 23 Apr 2023 13:22:04 +0200 Subject: [PATCH 14/55] Created CLI test for short URL importing --- .../test-cli/Command/ImportShortUrlsTest.php | 79 +++++++++++++++++++ 1 file changed, 79 insertions(+) create mode 100644 module/CLI/test-cli/Command/ImportShortUrlsTest.php diff --git a/module/CLI/test-cli/Command/ImportShortUrlsTest.php b/module/CLI/test-cli/Command/ImportShortUrlsTest.php new file mode 100644 index 00000000..3a710af0 --- /dev/null +++ b/module/CLI/test-cli/Command/ImportShortUrlsTest.php @@ -0,0 +1,79 @@ +tempCsvFile = tempnam(sys_get_temp_dir(), 'shlink_csv'); + if (! $this->tempCsvFile) { + return; + } + + $handle = fopen($this->tempCsvFile, 'w+'); + if (! $handle) { + $this->fail('It was not possible to open the temporary file to write CSV on it'); + } + + fwrite( + $handle, + <<tempCsvFile)) { + unlink($this->tempCsvFile); + } + } + + #[Test] + public function defaultDomainIsIgnoredWhenExplicitlyProvided(): void + { + if (! $this->tempCsvFile) { + $this->fail('It was not possible to create a temporary CSV file'); + } + + [$output] = $this->exec([ImportCommand::NAME, 'csv'], [$this->tempCsvFile, ';']); + + self::assertStringContainsString('https://shlink.io: Imported', $output); + self::assertStringContainsString('https://example.com: Imported', $output); + + [$listOutput1] = $this->exec( + [ListShortUrlsCommand::NAME, '--show-domain', '--search-term', 'testing-default-domain-import-1'], + ); + self::assertStringContainsString('DEFAULT', $listOutput1); + [$listOutput1] = $this->exec( + [ListShortUrlsCommand::NAME, '--show-domain', '--search-term', 'testing-default-domain-import-2'], + ); + self::assertStringContainsString('DEFAULT', $listOutput1); + } +} From 9fa291a32f7cdbfabe682524c3c82bc2a2063f36 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 23 Apr 2023 15:20:33 +0200 Subject: [PATCH 15/55] Update shlink-common --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 826f4896..2dd10f8a 100644 --- a/composer.json +++ b/composer.json @@ -45,7 +45,7 @@ "php-middleware/request-id": "^4.1", "pugx/shortid-php": "^1.1", "ramsey/uuid": "^4.7", - "shlinkio/shlink-common": "dev-main#9eecf8c as 5.5", + "shlinkio/shlink-common": "dev-main#29dd933 as 5.5", "shlinkio/shlink-config": "^2.4", "shlinkio/shlink-event-dispatcher": "dev-main#8c677ae as 3.0", "shlinkio/shlink-importer": "dev-main#6b63b12 as 5.1", From f129544f8391ea7c80ab65ae23eaa102153e4b57 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 23 Apr 2023 15:22:40 +0200 Subject: [PATCH 16/55] Update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f9c1dc57..40be8aad 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this ### Fixed * [#1760](https://github.com/shlinkio/shlink/issues/1760) Fix domain not being set to null when importing short URLs with default domain. +* Fix Shlink trying to connect to RabbitMQ even if configuration set to not connect. ## [3.5.4] - 2023-04-12 From a516ef691d47449a467b917eddf6d5db851ef35a Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 2 May 2023 08:43:14 +0200 Subject: [PATCH 17/55] Update to rr-cli 2.5, and do not generate config --- .github/workflows/ci.yml | 2 +- .gitignore | 1 - Dockerfile | 2 +- composer.json | 2 +- data/infra/roadrunner.Dockerfile | 2 +- 5 files changed, 4 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 15baf63e..73e85d13 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -70,7 +70,7 @@ jobs: php-version: ${{ matrix.php-version }} tools: composer - run: composer install --no-interaction --prefer-dist --ignore-platform-req=ext-openswoole - - run: ./vendor/bin/rr get --no-interaction --location bin/ && chmod +x bin/rr + - run: ./vendor/bin/rr get --no-interaction --no-config --location bin/ && chmod +x bin/rr - run: composer test:api:rr sqlite-db-tests: diff --git a/.gitignore b/.gitignore index daea5f2f..283d5b7f 100644 --- a/.gitignore +++ b/.gitignore @@ -1,5 +1,4 @@ .idea -bin/.rr.* bin/rr config/roadrunner/.pid build diff --git a/Dockerfile b/Dockerfile index 0dd2d8ff..c0120921 100644 --- a/Dockerfile +++ b/Dockerfile @@ -62,7 +62,7 @@ LABEL maintainer="Alejandro Celaya " COPY --from=builder /etc/shlink . RUN ln -s /etc/shlink/bin/cli /usr/local/bin/shlink && \ if [ "$SHLINK_RUNTIME" == 'rr' ]; then \ - php ./vendor/bin/rr get --no-interaction --location bin/ && chmod +x bin/rr ; \ + php ./vendor/bin/rr get --no-interaction --no-config --location bin/ && chmod +x bin/rr ; \ fi; # Expose default port diff --git a/composer.json b/composer.json index 2dd10f8a..719d392d 100644 --- a/composer.json +++ b/composer.json @@ -53,7 +53,7 @@ "shlinkio/shlink-ip-geolocation": "^3.2", "shlinkio/shlink-json": "^1.0", "spiral/roadrunner": "^2023.1", - "spiral/roadrunner-cli": "^2.4", + "spiral/roadrunner-cli": "^2.5", "spiral/roadrunner-http": "^3.0", "spiral/roadrunner-jobs": "^4.0", "symfony/console": "^6.2", diff --git a/data/infra/roadrunner.Dockerfile b/data/infra/roadrunner.Dockerfile index 383099e4..457a416f 100644 --- a/data/infra/roadrunner.Dockerfile +++ b/data/infra/roadrunner.Dockerfile @@ -71,6 +71,6 @@ CMD \ # Install dependencies if the vendor dir does not exist if [[ ! -d "./vendor" ]]; then /usr/local/bin/composer install ; fi && \ # Download roadrunner binary - if [[ ! -f "./bin/rr" ]]; then ./vendor/bin/rr get --no-interaction --location bin/ && chmod +x bin/rr ; fi && \ + if [[ ! -f "./bin/rr" ]]; then ./vendor/bin/rr get --no-interaction --no-config --location bin/ && chmod +x bin/rr ; fi && \ # This forces the app to be started every second until the exit code is 0 until ./bin/rr serve -c config/roadrunner/.rr.dev.yml; do sleep 1 ; done From b4b00a57c1a787a5f1ce2cb152f78dce788e0918 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 2 May 2023 19:40:23 +0200 Subject: [PATCH 18/55] Update chrome user agent used for anti-bots --- module/Core/src/Util/UrlValidator.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/module/Core/src/Util/UrlValidator.php b/module/Core/src/Util/UrlValidator.php index 762fdd9f..1ab3e8f8 100644 --- a/module/Core/src/Util/UrlValidator.php +++ b/module/Core/src/Util/UrlValidator.php @@ -27,7 +27,7 @@ class UrlValidator implements UrlValidatorInterface, RequestMethodInterface { private const MAX_REDIRECTS = 15; private const CHROME_USER_AGENT = 'Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) ' - . 'Chrome/108.0.0.0 Safari/537.36'; + . 'Chrome/112.0.0.0 Safari/537.36'; public function __construct(private ClientInterface $httpClient, private UrlShortenerOptions $options) { From 74069f2d24a632fd94ac46b4cf41e5a85d6f3163 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 2 May 2023 19:51:37 +0200 Subject: [PATCH 19/55] Skip API tests fetching Twitter during CI --- module/Rest/test-api/Action/CreateShortUrlTest.php | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/module/Rest/test-api/Action/CreateShortUrlTest.php b/module/Rest/test-api/Action/CreateShortUrlTest.php index 108a0f6f..54d1d45a 100644 --- a/module/Rest/test-api/Action/CreateShortUrlTest.php +++ b/module/Rest/test-api/Action/CreateShortUrlTest.php @@ -12,6 +12,7 @@ use Shlinkio\Shlink\TestUtils\ApiTest\ApiTestCase; use function Functional\map; use function range; +use function Shlinkio\Shlink\Config\env; use function sprintf; class CreateShortUrlTest extends ApiTestCase @@ -320,8 +321,14 @@ class CreateShortUrlTest extends ApiTestCase } #[Test, DataProvider('provideTwitterUrls')] - public function urlsWithBothProtectionCanBeShortenedWithUrlValidationEnabled(string $longUrl): void + public function urlsWithBotProtectionCanBeShortenedWithUrlValidationEnabled(string $longUrl): void { + // Requests to Twitter are randomly failing from GitHub actions. Let's skip this test there. + // This is a deprecated and low-used feature anyway. + if (env('CI', false)) { + $this->markTestSkipped(); + } + [$statusCode] = $this->createShortUrl(['longUrl' => $longUrl, 'validateUrl' => true]); self::assertEquals(self::STATUS_OK, $statusCode); } From e6a31b16eda7ae5f6b78726d7649fca457b97765 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 16 Apr 2023 13:10:24 +0200 Subject: [PATCH 20/55] Switch to roadrunner as default docker runtime --- .github/workflows/publish-docker-image.yml | 29 ++++++++++++---------- Dockerfile | 2 +- 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/.github/workflows/publish-docker-image.yml b/.github/workflows/publish-docker-image.yml index 7fb52fe1..f9e1630c 100644 --- a/.github/workflows/publish-docker-image.yml +++ b/.github/workflows/publish-docker-image.yml @@ -2,8 +2,6 @@ name: Build and publish docker image on: push: - branches: - - develop paths-ignore: - 'LICENSE' - '.*' @@ -12,24 +10,29 @@ on: - '*.yml*' - '*.json5' - '*.neon' + branches: + - develop tags: - 'v*' jobs: - build-openswoole: + build-image: + strategy: + matrix: + include: + - runtime: 'rr' + platforms: 'linux/arm64/v8,linux/amd64' + - runtime: 'rr' + tag-suffix: 'roadrunner' + platforms: 'linux/arm64/v8,linux/amd64' + - runtime: 'openswoole' + tag-suffix: 'openswoole' uses: shlinkio/github-actions/.github/workflows/docker-build-and-publish.yml@main secrets: inherit with: image-name: shlinkio/shlink version-arg-name: SHLINK_VERSION - - build-roadrunner: - uses: shlinkio/github-actions/.github/workflows/docker-build-and-publish.yml@main - secrets: inherit - with: - image-name: shlinkio/shlink - version-arg-name: SHLINK_VERSION - platforms: 'linux/arm64/v8,linux/amd64' - tags-suffix: roadrunner + platforms: ${{ matrix.platforms }} + tags-suffix: ${{ matrix.tag-suffix }} extra-build-args: | - SHLINK_RUNTIME=rr + SHLINK_RUNTIME=${{ matrix.runtime }} diff --git a/Dockerfile b/Dockerfile index c0120921..10897176 100644 --- a/Dockerfile +++ b/Dockerfile @@ -2,7 +2,7 @@ FROM php:8.2-alpine3.17 as base ARG SHLINK_VERSION=latest ENV SHLINK_VERSION ${SHLINK_VERSION} -ARG SHLINK_RUNTIME=openswoole +ARG SHLINK_RUNTIME=rr ENV SHLINK_RUNTIME ${SHLINK_RUNTIME} ENV OPENSWOOLE_VERSION 22.0.0 ENV PDO_SQLSRV_VERSION 5.10.1 From 28d93ea5e06c2f90440aa63b2068eb154abda958 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 16 Apr 2023 13:14:54 +0200 Subject: [PATCH 21/55] Update changelog --- CHANGELOG.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 40be8aad..ab9409d6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,13 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this ### Changed * [#1755](https://github.com/shlinkio/shlink/issues/1755) Update to roadrunner 2023 +* [#1745](https://github.com/shlinkio/shlink/issues/1745) Roadrunner is now the default docker runtime. + + There are now three different docker images published: + + * Versions without suffix (like `3.6.0`) will contain the default runtime, whichever it is. + * Versions with `-roadrunner` suffix (like `3.6.0-roadrunner`) will always use roadrunner as the runtime, even if default one changes in the future. + * Versions with `-openswoole` suffix (like `3.6.0-openswoole`) will always use openswoole as the runtime, even if default one changes in the future. ### Deprecated * *Nothing* From d4dea9a1d253d798f4180f5254a4035e2660edd7 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 6 May 2023 10:12:42 +0200 Subject: [PATCH 22/55] Update shlink-installer --- .github/workflows/ci.yml | 2 +- composer.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 73e85d13..bdc5a025 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -70,7 +70,7 @@ jobs: php-version: ${{ matrix.php-version }} tools: composer - run: composer install --no-interaction --prefer-dist --ignore-platform-req=ext-openswoole - - run: ./vendor/bin/rr get --no-interaction --no-config --location bin/ && chmod +x bin/rr + - run: ./vendor/bin/rr get --no-interaction --no-config --location bin/ && chmod +x bin/rr - run: composer test:api:rr sqlite-db-tests: diff --git a/composer.json b/composer.json index 719d392d..9cb587da 100644 --- a/composer.json +++ b/composer.json @@ -49,7 +49,7 @@ "shlinkio/shlink-config": "^2.4", "shlinkio/shlink-event-dispatcher": "dev-main#8c677ae as 3.0", "shlinkio/shlink-importer": "dev-main#6b63b12 as 5.1", - "shlinkio/shlink-installer": "^8.3", + "shlinkio/shlink-installer": "dev-develop#0e015f8 as 8.4", "shlinkio/shlink-ip-geolocation": "^3.2", "shlinkio/shlink-json": "^1.0", "spiral/roadrunner": "^2023.1", From a797b74a70e1e4f199f6a1776705eb2aac98469a Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 7 May 2023 13:18:19 +0200 Subject: [PATCH 23/55] Standardize logger for all Shlink execution contexts --- composer.json | 2 +- config/autoload/logger.global.php | 82 +++++++++++-------- config/autoload/logger.local.php.dist | 8 +- .../autoload/middleware-pipeline.global.php | 2 + config/roadrunner/.rr.dev.yml | 2 +- config/roadrunner/.rr.yml | 2 +- config/test/test_config.global.php | 1 + docker/config/shlink_in_docker.local.php | 4 +- 8 files changed, 56 insertions(+), 47 deletions(-) diff --git a/composer.json b/composer.json index 719d392d..2bce658c 100644 --- a/composer.json +++ b/composer.json @@ -45,7 +45,7 @@ "php-middleware/request-id": "^4.1", "pugx/shortid-php": "^1.1", "ramsey/uuid": "^4.7", - "shlinkio/shlink-common": "dev-main#29dd933 as 5.5", + "shlinkio/shlink-common": "dev-main#88a34f1 as 5.5", "shlinkio/shlink-config": "^2.4", "shlinkio/shlink-event-dispatcher": "dev-main#8c677ae as 3.0", "shlinkio/shlink-importer": "dev-main#6b63b12 as 5.1", diff --git a/config/autoload/logger.global.php b/config/autoload/logger.global.php index 2da1eda3..1820c480 100644 --- a/config/autoload/logger.global.php +++ b/config/autoload/logger.global.php @@ -4,51 +4,63 @@ declare(strict_types=1); namespace Shlinkio\Shlink; +use Laminas\ServiceManager\Factory\InvokableFactory; use Monolog\Level; use Monolog\Logger; use PhpMiddleware\RequestId; use Psr\Log\LoggerInterface; +use Psr\Log\NullLogger; use Shlinkio\Shlink\Common\Logger\LoggerFactory; use Shlinkio\Shlink\Common\Logger\LoggerType; +use Shlinkio\Shlink\Common\Middleware\AccessLogMiddleware; -$common = [ - 'level' => Level::Info->value, - 'processors' => [RequestId\MonologProcessor::class], - 'line_format' => '[%datetime%] [%extra.request_id%] %channel%.%level_name% - %message%', -]; +use function Shlinkio\Shlink\Config\runningInRoadRunner; -return [ +return (static function (): array { + $common = [ + 'level' => Level::Info->value, + 'processors' => [RequestId\MonologProcessor::class], + 'line_format' => '[%datetime%] [%extra.request_id%] %channel%.%level_name% - %message%', + ]; - 'logger' => [ - 'Shlink' => [ - 'type' => LoggerType::FILE->value, - ...$common, - ], - 'Access' => [ - 'type' => LoggerType::STREAM->value, - ...$common, - ], - ], + return [ - 'dependencies' => [ - 'factories' => [ - 'Logger_Shlink' => [LoggerFactory::class, 'Shlink'], - 'Logger_Access' => [LoggerFactory::class, 'Access'], - ], - 'aliases' => [ - 'logger' => 'Logger_Shlink', - Logger::class => 'Logger_Shlink', - LoggerInterface::class => 'Logger_Shlink', - ], - ], - - 'mezzio-swoole' => [ - 'swoole-http-server' => [ - 'logger' => [ - 'logger-name' => 'Logger_Access', - 'format' => '%u "%r" %>s %B', + 'logger' => [ + 'Shlink' => [ + 'type' => LoggerType::FILE->value, + ...$common, + ], + 'Access' => [ + 'type' => LoggerType::STREAM->value, + 'destination' => 'php://stderr', + 'add_new_line' => ! runningInRoadRunner(), + ...$common, ], ], - ], -]; + 'dependencies' => [ + 'factories' => [ + 'Logger_Shlink' => [LoggerFactory::class, 'Shlink'], + 'Logger_Access' => [LoggerFactory::class, 'Access'], + NullLogger::class => InvokableFactory::class, + ], + 'aliases' => [ + 'logger' => 'Logger_Shlink', + Logger::class => 'Logger_Shlink', + LoggerInterface::class => 'Logger_Shlink', + AccessLogMiddleware::LOGGER_SERVICE_NAME => 'Logger_Access', + ], + ], + + 'mezzio-swoole' => [ + 'swoole-http-server' => [ + 'logger' => [ + // Let's disable mezio-swoole access logging, so that we can provide our own implementation, + // consistent for roadrunner and openswoole + 'logger-name' => NullLogger::class, + ], + ], + ], + + ]; +})(); diff --git a/config/autoload/logger.local.php.dist b/config/autoload/logger.local.php.dist index 919f6cdb..fe2c8c54 100644 --- a/config/autoload/logger.local.php.dist +++ b/config/autoload/logger.local.php.dist @@ -5,16 +5,12 @@ declare(strict_types=1); use Monolog\Level; use Shlinkio\Shlink\Common\Logger\LoggerType; -use function Shlinkio\Shlink\Config\runningInOpenswoole; - -$logToStream = runningInOpenswoole(); - return [ 'logger' => [ 'Shlink' => [ - // For openswoole, send logs as stream - 'type' => $logToStream ? LoggerType::STREAM->value : LoggerType::FILE->value, + 'type' => LoggerType::STREAM->value, + 'destination' => 'php://stderr', 'level' => Level::Debug->value, ], ], diff --git a/config/autoload/middleware-pipeline.global.php b/config/autoload/middleware-pipeline.global.php index 25db6b7b..cb8045e9 100644 --- a/config/autoload/middleware-pipeline.global.php +++ b/config/autoload/middleware-pipeline.global.php @@ -9,6 +9,7 @@ use Mezzio\ProblemDetails; use Mezzio\Router; use PhpMiddleware\RequestId\RequestIdMiddleware; use RKA\Middleware\IpAddress; +use Shlinkio\Shlink\Common\Middleware\AccessLogMiddleware; use Shlinkio\Shlink\Common\Middleware\ContentLengthMiddleware; return [ @@ -16,6 +17,7 @@ return [ 'middleware_pipeline' => [ 'error-handler' => [ 'middleware' => [ + AccessLogMiddleware::class, ContentLengthMiddleware::class, RequestIdMiddleware::class, ErrorHandler::class, diff --git a/config/roadrunner/.rr.dev.yml b/config/roadrunner/.rr.dev.yml index cc0bbf29..9038c07e 100644 --- a/config/roadrunner/.rr.dev.yml +++ b/config/roadrunner/.rr.dev.yml @@ -31,7 +31,7 @@ logs: mode: development channels: http: - level: debug + mode: 'off' # Disable logging as Shlink handles it internally server: level: debug metrics: diff --git a/config/roadrunner/.rr.yml b/config/roadrunner/.rr.yml index d44801ee..b4074f96 100644 --- a/config/roadrunner/.rr.yml +++ b/config/roadrunner/.rr.yml @@ -31,6 +31,6 @@ logs: mode: production channels: http: - level: info # Log all http requests, set to info to disable + mode: 'off' # Disable logging as Shlink handles it internally server: level: debug # Everything written to worker stderr is logged diff --git a/config/test/test_config.global.php b/config/test/test_config.global.php index ac62f8a6..1beed0e3 100644 --- a/config/test/test_config.global.php +++ b/config/test/test_config.global.php @@ -121,6 +121,7 @@ $buildTestLoggerConfig = static fn (string $filename) => [ 'level' => Level::Debug->value, 'type' => LoggerType::STREAM->value, 'destination' => sprintf('data/log/api-tests/%s', $filename), + 'add_new_line' => true, ]; return [ diff --git a/docker/config/shlink_in_docker.local.php b/docker/config/shlink_in_docker.local.php index 9dc99351..2d5d6a06 100644 --- a/docker/config/shlink_in_docker.local.php +++ b/docker/config/shlink_in_docker.local.php @@ -6,14 +6,12 @@ namespace Shlinkio\Shlink; use Shlinkio\Shlink\Common\Logger\LoggerType; -use function Shlinkio\Shlink\Config\runningInRoadRunner; - return [ 'logger' => [ 'Shlink' => [ 'type' => LoggerType::STREAM->value, - 'destination' => runningInRoadRunner() ? 'php://stderr' : 'php://stdout', + 'destination' => 'php://stderr', ], ], From 3b4c1501f3c70f6c4b74edb2f3426abe2c270073 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 7 May 2023 17:13:26 +0200 Subject: [PATCH 24/55] Set platforms to be used for openswoole docker image --- .github/workflows/publish-docker-image.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/publish-docker-image.yml b/.github/workflows/publish-docker-image.yml index f9e1630c..6beb9ac1 100644 --- a/.github/workflows/publish-docker-image.yml +++ b/.github/workflows/publish-docker-image.yml @@ -27,6 +27,7 @@ jobs: platforms: 'linux/arm64/v8,linux/amd64' - runtime: 'openswoole' tag-suffix: 'openswoole' + platforms: 'linux/arm/v7,linux/arm64/v8,linux/amd64' uses: shlinkio/github-actions/.github/workflows/docker-build-and-publish.yml@main secrets: inherit with: From 2573c2bf987f4fdc1811f9eb4402bd584bec6b1f Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 14 May 2023 11:56:49 +0200 Subject: [PATCH 25/55] Update roadrunner config --- config/roadrunner/.rr.dev.yml | 15 +++------------ config/roadrunner/.rr.yml | 2 +- 2 files changed, 4 insertions(+), 13 deletions(-) diff --git a/config/roadrunner/.rr.dev.yml b/config/roadrunner/.rr.dev.yml index 9038c07e..a69a805f 100644 --- a/config/roadrunner/.rr.dev.yml +++ b/config/roadrunner/.rr.dev.yml @@ -1,4 +1,4 @@ -version: '2.7' +version: '3.0' rpc: listen: tcp://127.0.0.1:6001 @@ -14,10 +14,12 @@ http: forbid: ['.php', '.htaccess'] pool: num_workers: 1 + debug: true jobs: pool: num_workers: 1 + debug: true timeout: 300 consume: ['shlink'] pipelines: @@ -36,14 +38,3 @@ logs: level: debug metrics: level: debug - -reload: - interval: 1s - patterns: ['.php'] - services: - http: - dirs: ['../../bin', '../../config', '../../data/migrations', '../../module', '../../vendor'] - recursive: true - jobs: - dirs: ['../../bin', '../../config', '../../data/migrations', '../../module', '../../vendor'] - recursive: true diff --git a/config/roadrunner/.rr.yml b/config/roadrunner/.rr.yml index b4074f96..8d1344d7 100644 --- a/config/roadrunner/.rr.yml +++ b/config/roadrunner/.rr.yml @@ -1,4 +1,4 @@ -version: '2.7' +version: '3.0' rpc: listen: tcp://127.0.0.1:6001 From 84a7981dfaf2c0386625c8d2ed0d63f4ae9dc908 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 14 May 2023 12:00:08 +0200 Subject: [PATCH 26/55] Create REST action to delete short URL visits --- config/autoload/routes.config.php | 1 + module/Core/config/dependencies.config.php | 9 +++++ module/Core/src/Model/BulkDeleteResult.php | 17 ++++++++++ .../src/ShortUrl/ShortUrlVisitsDeleter.php | 25 ++++++++++++++ .../ShortUrlVisitsDeleterInterface.php | 14 ++++++++ .../Repository/VisitDeleterRepository.php | 18 ++++++++++ .../VisitDeleterRepositoryInterface.php | 13 ++++++++ module/Rest/config/dependencies.config.php | 2 ++ .../ShortUrl/DeleteShortUrlVisitsAction.php | 33 +++++++++++++++++++ 9 files changed, 132 insertions(+) create mode 100644 module/Core/src/Model/BulkDeleteResult.php create mode 100644 module/Core/src/ShortUrl/ShortUrlVisitsDeleter.php create mode 100644 module/Core/src/ShortUrl/ShortUrlVisitsDeleterInterface.php create mode 100644 module/Core/src/Visit/Repository/VisitDeleterRepository.php create mode 100644 module/Core/src/Visit/Repository/VisitDeleterRepositoryInterface.php create mode 100644 module/Rest/src/Action/ShortUrl/DeleteShortUrlVisitsAction.php diff --git a/config/autoload/routes.config.php b/config/autoload/routes.config.php index c887d5b7..93464519 100644 --- a/config/autoload/routes.config.php +++ b/config/autoload/routes.config.php @@ -53,6 +53,7 @@ return (static function (): array { ]), Action\ShortUrl\EditShortUrlAction::getRouteDef([$dropDomainMiddleware]), Action\ShortUrl\DeleteShortUrlAction::getRouteDef([$dropDomainMiddleware]), + Action\ShortUrl\DeleteShortUrlVisitsAction::getRouteDef([$dropDomainMiddleware]), Action\ShortUrl\ResolveShortUrlAction::getRouteDef([$dropDomainMiddleware]), Action\ShortUrl\ListShortUrlsAction::getRouteDef(), diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index 008db777..8e3e9a52 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -38,6 +38,7 @@ return [ ShortUrl\ShortUrlListService::class => ConfigAbstractFactory::class, ShortUrl\DeleteShortUrlService::class => ConfigAbstractFactory::class, ShortUrl\ShortUrlResolver::class => ConfigAbstractFactory::class, + ShortUrl\ShortUrlVisitsDeleter::class => ConfigAbstractFactory::class, ShortUrl\Helper\ShortCodeUniquenessHelper::class => ConfigAbstractFactory::class, ShortUrl\Resolver\PersistenceShortUrlRelationResolver::class => ConfigAbstractFactory::class, ShortUrl\Helper\ShortUrlStringifier::class => ConfigAbstractFactory::class, @@ -69,6 +70,10 @@ return [ EntityRepositoryFactory::class, Visit\Entity\Visit::class, ], + Visit\Repository\VisitDeleterRepository::class => [ + EntityRepositoryFactory::class, + Visit\Entity\Visit::class, + ], Util\UrlValidator::class => ConfigAbstractFactory::class, Util\DoctrineBatchHelper::class => ConfigAbstractFactory::class, @@ -137,6 +142,10 @@ return [ ShortUrl\ShortUrlResolver::class, ], ShortUrl\ShortUrlResolver::class => ['em', Options\UrlShortenerOptions::class], + ShortUrl\ShortUrlVisitsDeleter::class => [ + Visit\Repository\VisitDeleterRepository::class, + ShortUrl\ShortUrlResolver::class, + ], ShortUrl\Helper\ShortCodeUniquenessHelper::class => ['em', Options\UrlShortenerOptions::class], Domain\DomainService::class => ['em', 'config.url_shortener.domain.hostname'], diff --git a/module/Core/src/Model/BulkDeleteResult.php b/module/Core/src/Model/BulkDeleteResult.php new file mode 100644 index 00000000..b3b0e756 --- /dev/null +++ b/module/Core/src/Model/BulkDeleteResult.php @@ -0,0 +1,17 @@ + $this->affectedItems]; + } +} diff --git a/module/Core/src/ShortUrl/ShortUrlVisitsDeleter.php b/module/Core/src/ShortUrl/ShortUrlVisitsDeleter.php new file mode 100644 index 00000000..097c8875 --- /dev/null +++ b/module/Core/src/ShortUrl/ShortUrlVisitsDeleter.php @@ -0,0 +1,25 @@ +resolver->resolveShortUrl($identifier, $apiKey); + return new BulkDeleteResult($this->repository->deleteShortUrlVisits($identifier, $apiKey)); + } +} diff --git a/module/Core/src/ShortUrl/ShortUrlVisitsDeleterInterface.php b/module/Core/src/ShortUrl/ShortUrlVisitsDeleterInterface.php new file mode 100644 index 00000000..dc29ef94 --- /dev/null +++ b/module/Core/src/ShortUrl/ShortUrlVisitsDeleterInterface.php @@ -0,0 +1,14 @@ + ConfigAbstractFactory::class, Action\ShortUrl\ResolveShortUrlAction::class => ConfigAbstractFactory::class, Action\ShortUrl\ListShortUrlsAction::class => ConfigAbstractFactory::class, + Action\ShortUrl\DeleteShortUrlVisitsAction::class => ConfigAbstractFactory::class, Action\Visit\ShortUrlVisitsAction::class => ConfigAbstractFactory::class, Action\Visit\TagVisitsAction::class => ConfigAbstractFactory::class, Action\Visit\DomainVisitsAction::class => ConfigAbstractFactory::class, @@ -94,6 +95,7 @@ return [ ShortUrl\ShortUrlListService::class, ShortUrlDataTransformer::class, ], + Action\ShortUrl\DeleteShortUrlVisitsAction::class => [ShortUrl\ShortUrlVisitsDeleter::class], Action\Tag\ListTagsAction::class => [TagService::class], Action\Tag\TagsStatsAction::class => [TagService::class], Action\Tag\DeleteTagsAction::class => [TagService::class], diff --git a/module/Rest/src/Action/ShortUrl/DeleteShortUrlVisitsAction.php b/module/Rest/src/Action/ShortUrl/DeleteShortUrlVisitsAction.php new file mode 100644 index 00000000..c9eaf958 --- /dev/null +++ b/module/Rest/src/Action/ShortUrl/DeleteShortUrlVisitsAction.php @@ -0,0 +1,33 @@ +deleter->deleteShortUrlVisits($identifier, $apiKey); + + return new JsonResponse($result->toArray('deletedVisits')); + } +} From ffc0555c7c01fa281a1999f8cf05771f76eae66f Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 14 May 2023 12:15:35 +0200 Subject: [PATCH 27/55] Create DeleteShortUrlVisitsActionTest --- .../DeleteShortUrlVisitsActionTest.php | 56 +++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 module/Rest/test/Action/ShortUrl/DeleteShortUrlVisitsActionTest.php diff --git a/module/Rest/test/Action/ShortUrl/DeleteShortUrlVisitsActionTest.php b/module/Rest/test/Action/ShortUrl/DeleteShortUrlVisitsActionTest.php new file mode 100644 index 00000000..8ff727c6 --- /dev/null +++ b/module/Rest/test/Action/ShortUrl/DeleteShortUrlVisitsActionTest.php @@ -0,0 +1,56 @@ +deleter = $this->createMock(ShortUrlVisitsDeleterInterface::class); + $this->action = new DeleteShortUrlVisitsAction($this->deleter); + } + + #[Test, DataProvider('provideVisitsCounts')] + public function visitsAreDeletedForShortUrl(int $visitsCount): void + { + $apiKey = ApiKey::create(); + $request = ServerRequestFactory::fromGlobals()->withAttribute(ApiKey::class, $apiKey) + ->withAttribute('shortCode', 'foo'); + + $this->deleter->expects($this->once())->method('deleteShortUrlVisits')->with( + ShortUrlIdentifier::fromShortCodeAndDomain('foo'), + $apiKey, + )->willReturn(new BulkDeleteResult($visitsCount)); + + /** @var JsonResponse $resp */ + $resp = $this->action->handle($request); + $payload = $resp->getPayload(); + + self::assertEquals(['deletedVisits' => $visitsCount], $payload); + } + + public static function provideVisitsCounts(): iterable + { + yield '1' => [1]; + yield '0' => [0]; + yield '300' => [300]; + yield '1234' => [1234]; + } +} From 69ff7de481e8c4878c62df345214068923002b3a Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 14 May 2023 12:32:54 +0200 Subject: [PATCH 28/55] Create ShortUrlVisitsDeleterTest --- .../src/ShortUrl/ShortUrlVisitsDeleter.php | 4 ++ .../ShortUrlVisitsDeleterInterface.php | 4 ++ .../ShortUrl/ShortUrlVisitsDeleterTest.php | 51 +++++++++++++++++++ 3 files changed, 59 insertions(+) create mode 100644 module/Core/test/ShortUrl/ShortUrlVisitsDeleterTest.php diff --git a/module/Core/src/ShortUrl/ShortUrlVisitsDeleter.php b/module/Core/src/ShortUrl/ShortUrlVisitsDeleter.php index 097c8875..5442af7c 100644 --- a/module/Core/src/ShortUrl/ShortUrlVisitsDeleter.php +++ b/module/Core/src/ShortUrl/ShortUrlVisitsDeleter.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\ShortUrl; +use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; use Shlinkio\Shlink\Core\Model\BulkDeleteResult; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Visit\Repository\VisitDeleterRepositoryInterface; @@ -17,6 +18,9 @@ class ShortUrlVisitsDeleter implements ShortUrlVisitsDeleterInterface ) { } + /** + * @throws ShortUrlNotFoundException + */ public function deleteShortUrlVisits(ShortUrlIdentifier $identifier, ?ApiKey $apiKey): BulkDeleteResult { $this->resolver->resolveShortUrl($identifier, $apiKey); diff --git a/module/Core/src/ShortUrl/ShortUrlVisitsDeleterInterface.php b/module/Core/src/ShortUrl/ShortUrlVisitsDeleterInterface.php index dc29ef94..b0ac0e6a 100644 --- a/module/Core/src/ShortUrl/ShortUrlVisitsDeleterInterface.php +++ b/module/Core/src/ShortUrl/ShortUrlVisitsDeleterInterface.php @@ -4,11 +4,15 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\ShortUrl; +use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; use Shlinkio\Shlink\Core\Model\BulkDeleteResult; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Rest\Entity\ApiKey; interface ShortUrlVisitsDeleterInterface { + /** + * @throws ShortUrlNotFoundException + */ public function deleteShortUrlVisits(ShortUrlIdentifier $identifier, ?ApiKey $apiKey): BulkDeleteResult; } diff --git a/module/Core/test/ShortUrl/ShortUrlVisitsDeleterTest.php b/module/Core/test/ShortUrl/ShortUrlVisitsDeleterTest.php new file mode 100644 index 00000000..461fc78b --- /dev/null +++ b/module/Core/test/ShortUrl/ShortUrlVisitsDeleterTest.php @@ -0,0 +1,51 @@ +repository = $this->createMock(VisitDeleterRepositoryInterface::class); + $this->resolver = $this->createMock(ShortUrlResolverInterface::class); + + $this->deleter = new ShortUrlVisitsDeleter($this->repository, $this->resolver); + } + + #[Test, DataProvider('provideVisitsCounts')] + public function returnsDeletedVisitsFromRepo(int $visitsCount): void + { + $identifier = ShortUrlIdentifier::fromShortCodeAndDomain(''); + + $this->resolver->expects($this->once())->method('resolveShortUrl')->with($identifier, null); + $this->repository->expects($this->once())->method('deleteShortUrlVisits')->with($identifier, null)->willReturn( + $visitsCount, + ); + + $result = $this->deleter->deleteShortUrlVisits($identifier, null); + + self::assertEquals($visitsCount, $result->affectedItems); + } + + public static function provideVisitsCounts(): iterable + { + yield '45' => [45]; + yield '5000' => [5000]; + yield '0' => [0]; + } +} From 531a19dde936873f20b331100e7ec568078152a9 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 14 May 2023 13:04:17 +0200 Subject: [PATCH 29/55] Refactor short URL visits deletion layers --- module/Core/src/ShortUrl/ShortUrlVisitsDeleter.php | 4 ++-- .../Visit/Repository/VisitDeleterRepository.php | 14 +++++++++----- .../Repository/VisitDeleterRepositoryInterface.php | 5 ++--- .../test/ShortUrl/ShortUrlVisitsDeleterTest.php | 8 ++++++-- 4 files changed, 19 insertions(+), 12 deletions(-) diff --git a/module/Core/src/ShortUrl/ShortUrlVisitsDeleter.php b/module/Core/src/ShortUrl/ShortUrlVisitsDeleter.php index 5442af7c..c202c5c2 100644 --- a/module/Core/src/ShortUrl/ShortUrlVisitsDeleter.php +++ b/module/Core/src/ShortUrl/ShortUrlVisitsDeleter.php @@ -23,7 +23,7 @@ class ShortUrlVisitsDeleter implements ShortUrlVisitsDeleterInterface */ public function deleteShortUrlVisits(ShortUrlIdentifier $identifier, ?ApiKey $apiKey): BulkDeleteResult { - $this->resolver->resolveShortUrl($identifier, $apiKey); - return new BulkDeleteResult($this->repository->deleteShortUrlVisits($identifier, $apiKey)); + $shortUrl = $this->resolver->resolveShortUrl($identifier, $apiKey); + return new BulkDeleteResult($this->repository->deleteShortUrlVisits($shortUrl)); } } diff --git a/module/Core/src/Visit/Repository/VisitDeleterRepository.php b/module/Core/src/Visit/Repository/VisitDeleterRepository.php index 79a91f9d..602ba576 100644 --- a/module/Core/src/Visit/Repository/VisitDeleterRepository.php +++ b/module/Core/src/Visit/Repository/VisitDeleterRepository.php @@ -5,14 +5,18 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Visit\Repository; use Happyr\DoctrineSpecification\Repository\EntitySpecificationRepository; -use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlIdentifier; -use Shlinkio\Shlink\Rest\Entity\ApiKey; +use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; +use Shlinkio\Shlink\Core\Visit\Entity\Visit; class VisitDeleterRepository extends EntitySpecificationRepository implements VisitDeleterRepositoryInterface { - public function deleteShortUrlVisits(ShortUrlIdentifier $identifier, ?ApiKey $apiKey): int + public function deleteShortUrlVisits(ShortUrl $shortUrl): int { - // TODO: Implement deleteShortUrlVisits() method. - return 0; + $qb = $this->getEntityManager()->createQueryBuilder(); + $qb->delete(Visit::class, 'v') + ->where($qb->expr()->eq('v.shortUrl', ':shortUrl')) + ->setParameter('shortUrl', $shortUrl); + + return $qb->getQuery()->execute(); } } diff --git a/module/Core/src/Visit/Repository/VisitDeleterRepositoryInterface.php b/module/Core/src/Visit/Repository/VisitDeleterRepositoryInterface.php index be2df6a3..61a8af9b 100644 --- a/module/Core/src/Visit/Repository/VisitDeleterRepositoryInterface.php +++ b/module/Core/src/Visit/Repository/VisitDeleterRepositoryInterface.php @@ -4,10 +4,9 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Visit\Repository; -use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlIdentifier; -use Shlinkio\Shlink\Rest\Entity\ApiKey; +use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; interface VisitDeleterRepositoryInterface { - public function deleteShortUrlVisits(ShortUrlIdentifier $identifier, ?ApiKey $apiKey): int; + public function deleteShortUrlVisits(ShortUrl $shortUrl): int; } diff --git a/module/Core/test/ShortUrl/ShortUrlVisitsDeleterTest.php b/module/Core/test/ShortUrl/ShortUrlVisitsDeleterTest.php index 461fc78b..e1690a5b 100644 --- a/module/Core/test/ShortUrl/ShortUrlVisitsDeleterTest.php +++ b/module/Core/test/ShortUrl/ShortUrlVisitsDeleterTest.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\ShortUrl\Entity\ShortUrl; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\ShortUrl\ShortUrlResolverInterface; use Shlinkio\Shlink\Core\ShortUrl\ShortUrlVisitsDeleter; @@ -31,9 +32,12 @@ class ShortUrlVisitsDeleterTest extends TestCase public function returnsDeletedVisitsFromRepo(int $visitsCount): void { $identifier = ShortUrlIdentifier::fromShortCodeAndDomain(''); + $shortUrl = ShortUrl::withLongUrl('https://example.com'); - $this->resolver->expects($this->once())->method('resolveShortUrl')->with($identifier, null); - $this->repository->expects($this->once())->method('deleteShortUrlVisits')->with($identifier, null)->willReturn( + $this->resolver->expects($this->once())->method('resolveShortUrl')->with($identifier, null)->willReturn( + $shortUrl, + ); + $this->repository->expects($this->once())->method('deleteShortUrlVisits')->with($shortUrl)->willReturn( $visitsCount, ); From b8143a5bb472213e5b202b4e747c0acf525cb1ab Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 14 May 2023 13:04:45 +0200 Subject: [PATCH 30/55] Create VisitDeleterRepositoryTest --- .../Repository/VisitDeleterRepositoryTest.php | 63 +++++++++++++++++++ 1 file changed, 63 insertions(+) create mode 100644 module/Core/test-db/Visit/Repository/VisitDeleterRepositoryTest.php diff --git a/module/Core/test-db/Visit/Repository/VisitDeleterRepositoryTest.php b/module/Core/test-db/Visit/Repository/VisitDeleterRepositoryTest.php new file mode 100644 index 00000000..1598fc94 --- /dev/null +++ b/module/Core/test-db/Visit/Repository/VisitDeleterRepositoryTest.php @@ -0,0 +1,63 @@ +getEntityManager(); + $this->repo = new VisitDeleterRepository($em, $em->getClassMetadata(Visit::class)); + } + + #[Test] + public function deletesExpectedVisits(): void + { + $shortUrl1 = ShortUrl::withLongUrl('https://foo.com'); + $this->getEntityManager()->persist($shortUrl1); + $this->getEntityManager()->persist(Visit::forValidShortUrl($shortUrl1, Visitor::emptyInstance())); + $this->getEntityManager()->persist(Visit::forValidShortUrl($shortUrl1, Visitor::emptyInstance())); + + $shortUrl2 = ShortUrl::create(ShortUrlCreation::fromRawData([ + ShortUrlInputFilter::LONG_URL => 'https://foo.com', + ShortUrlInputFilter::DOMAIN => 's.test', + ShortUrlInputFilter::CUSTOM_SLUG => 'foo', + ]), new PersistenceShortUrlRelationResolver($this->getEntityManager())); + $this->getEntityManager()->persist($shortUrl2); + $this->getEntityManager()->persist(Visit::forValidShortUrl($shortUrl2, Visitor::emptyInstance())); + $this->getEntityManager()->persist(Visit::forValidShortUrl($shortUrl2, Visitor::emptyInstance())); + $this->getEntityManager()->persist(Visit::forValidShortUrl($shortUrl2, Visitor::emptyInstance())); + $this->getEntityManager()->persist(Visit::forValidShortUrl($shortUrl2, Visitor::emptyInstance())); + + $shortUrl3 = ShortUrl::create(ShortUrlCreation::fromRawData([ + ShortUrlInputFilter::LONG_URL => 'https://foo.com', + ShortUrlInputFilter::CUSTOM_SLUG => 'foo', + ]), new PersistenceShortUrlRelationResolver($this->getEntityManager())); + $this->getEntityManager()->persist($shortUrl3); + $this->getEntityManager()->persist(Visit::forValidShortUrl($shortUrl3, Visitor::emptyInstance())); + + $this->getEntityManager()->flush(); + + self::assertEquals(0, $this->repo->deleteShortUrlVisits(ShortUrl::withLongUrl('https://invalid')->setId('99'))); + self::assertEquals(2, $this->repo->deleteShortUrlVisits($shortUrl1)); + self::assertEquals(0, $this->repo->deleteShortUrlVisits($shortUrl1)); + self::assertEquals(4, $this->repo->deleteShortUrlVisits($shortUrl2)); + self::assertEquals(0, $this->repo->deleteShortUrlVisits($shortUrl2)); + self::assertEquals(1, $this->repo->deleteShortUrlVisits($shortUrl3)); + self::assertEquals(0, $this->repo->deleteShortUrlVisits($shortUrl3)); + } +} From 0365728337f957fbd61b3bb73f8cb3c2acb505d0 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 14 May 2023 13:35:15 +0200 Subject: [PATCH 31/55] Create DeleteShortUrlVisitsTest --- .../Action/DeleteShortUrlVisitsTest.php | 86 +++++++++++++++++++ 1 file changed, 86 insertions(+) create mode 100644 module/Rest/test-api/Action/DeleteShortUrlVisitsTest.php diff --git a/module/Rest/test-api/Action/DeleteShortUrlVisitsTest.php b/module/Rest/test-api/Action/DeleteShortUrlVisitsTest.php new file mode 100644 index 00000000..045f2c9a --- /dev/null +++ b/module/Rest/test-api/Action/DeleteShortUrlVisitsTest.php @@ -0,0 +1,86 @@ +getTotalVisits()); + self::assertEquals(3, $this->getOrphanVisits()); + + $resp = $this->callApiWithKey(self::METHOD_DELETE, '/short-urls/abc123/visits'); + $payload = $this->getJsonResponsePayload($resp); + + self::assertEquals(200, $resp->getStatusCode()); + self::assertEquals(3, $payload['deletedVisits']); + self::assertEquals(4, $this->getTotalVisits()); + self::assertEquals(3, $this->getOrphanVisits()); + } + + private function getTotalVisits(): int + { + $resp = $this->callApiWithKey(self::METHOD_GET, '/visits/non-orphan'); + $payload = $this->getJsonResponsePayload($resp); + + return $payload['visits']['pagination']['totalItems']; + } + + private function getOrphanVisits(): int + { + $resp = $this->callApiWithKey(self::METHOD_GET, '/visits/orphan'); + $payload = $this->getJsonResponsePayload($resp); + + return $payload['visits']['pagination']['totalItems']; + } + + #[Test, DataProvider('provideInvalidShortUrls')] + public function returnsErrorForInvalidShortUrls(string $uri, array $options, string $expectedError): void + { + $resp = $this->callApiWithKey(self::METHOD_DELETE, '/rest/v3' . $uri, $options); + $payload = $this->getJsonResponsePayload($resp); + + self::assertEquals(404, $resp->getStatusCode()); + self::assertEquals($expectedError, $payload['detail']); + self::assertEquals('https://shlink.io/api/error/short-url-not-found', $payload['type']); + } + + public static function provideInvalidShortUrls(): iterable + { + yield 'not exists' => [ + '/short-urls/does-not-exist/visits', + [], + 'No URL found with short code "does-not-exist"', + ]; + yield 'needs domain' => [ + '/short-urls/custom-with-domain/visits', + [], + 'No URL found with short code "custom-with-domain"', + ]; + yield 'invalid domain' => [ + '/short-urls/abc123/visits', + [RequestOptions::QUERY => ['domain' => 'ff.test']], + 'No URL found with short code "abc123" for domain "ff.test"', + ]; + yield 'wrong domain' => [ + '/short-urls/custom-with-domain/visits', + [RequestOptions::QUERY => ['domain' => 'ff.test']], + 'No URL found with short code "custom-with-domain" for domain "ff.test"', + ]; + } + + #[Test] + public function cannotDeleteVisitsForShortUrlWithWrongApiKeyPermissions(): void + { + $resp = $this->callApiWithKey(self::METHOD_DELETE, '/short-urls/abc123/visits', [], 'domain_api_key'); + self::assertEquals(404, $resp->getStatusCode()); + } +} From 3cf253fd0f5264e4532de27f65e79ce47b37880f Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 14 May 2023 18:25:21 +0200 Subject: [PATCH 32/55] Document short URLs visits deletion endpoint --- docs/swagger/parameters/shortCode.json | 9 ++ .../paths/v1_short-urls_{shortCode}.json | 24 +----- .../v1_short-urls_{shortCode}_visits.json | 82 +++++++++++++++++-- docs/swagger/paths/{shortCode}.json | 8 +- docs/swagger/paths/{shortCode}_qr-code.json | 8 +- docs/swagger/paths/{shortCode}_track.json | 8 +- 6 files changed, 90 insertions(+), 49 deletions(-) create mode 100644 docs/swagger/parameters/shortCode.json diff --git a/docs/swagger/parameters/shortCode.json b/docs/swagger/parameters/shortCode.json new file mode 100644 index 00000000..f8eddca2 --- /dev/null +++ b/docs/swagger/parameters/shortCode.json @@ -0,0 +1,9 @@ +{ + "name": "shortCode", + "in": "path", + "description": "The short code for the short URL.", + "required": true, + "schema": { + "type": "string" + } +} diff --git a/docs/swagger/paths/v1_short-urls_{shortCode}.json b/docs/swagger/paths/v1_short-urls_{shortCode}.json index e639f362..408d166c 100644 --- a/docs/swagger/paths/v1_short-urls_{shortCode}.json +++ b/docs/swagger/paths/v1_short-urls_{shortCode}.json @@ -11,13 +11,7 @@ "$ref": "../parameters/version.json" }, { - "name": "shortCode", - "in": "path", - "description": "The short code to resolve.", - "required": true, - "schema": { - "type": "string" - } + "$ref": "../parameters/shortCode.json" }, { "$ref": "../parameters/domain.json" @@ -127,13 +121,7 @@ "$ref": "../parameters/version.json" }, { - "name": "shortCode", - "in": "path", - "description": "The short code to edit.", - "required": true, - "schema": { - "type": "string" - } + "$ref": "../parameters/shortCode.json" }, { "$ref": "../parameters/domain.json" @@ -295,13 +283,7 @@ "$ref": "../parameters/version.json" }, { - "name": "shortCode", - "in": "path", - "description": "The short code to edit.", - "required": true, - "schema": { - "type": "string" - } + "$ref": "../parameters/shortCode.json" }, { "$ref": "../parameters/domain.json" diff --git a/docs/swagger/paths/v1_short-urls_{shortCode}_visits.json b/docs/swagger/paths/v1_short-urls_{shortCode}_visits.json index e86bb698..2f102711 100644 --- a/docs/swagger/paths/v1_short-urls_{shortCode}_visits.json +++ b/docs/swagger/paths/v1_short-urls_{shortCode}_visits.json @@ -11,13 +11,7 @@ "$ref": "../parameters/version.json" }, { - "name": "shortCode", - "in": "path", - "description": "The short code for the short URL from which we want to get the visits.", - "required": true, - "schema": { - "type": "string" - } + "$ref": "../parameters/shortCode.json" }, { "$ref": "../parameters/domain.json" @@ -172,5 +166,79 @@ } } } + }, + + "delete": { + "operationId": "deleteShortUrlVisits", + "tags": [ + "Visits" + ], + "summary": "Delete visits for short URL", + "description": "Delete all existing visits on the short URL behind provided short code.", + "parameters": [ + { + "$ref": "../parameters/version.json" + }, + { + "$ref": "../parameters/shortCode.json" + }, + { + "$ref": "../parameters/domain.json" + } + ], + "security": [ + { + "ApiKey": [] + } + ], + "responses": { + "200": { + "description": "Deleted visits", + "content": { + "application/json": { + "schema": { + "type": "object", + "properties": { + "deletedVisits": { + "description": "Amount of affected visits", + "type": "number" + } + } + }, + "example": { + "deletedVisits": 536 + } + } + } + }, + "404": { + "description": "The short code does not belong to any short URL.", + "content": { + "application/problem+json": { + "schema": { + "$ref": "../definitions/Error.json" + }, + "examples": { + "Short URL not found with API v3 and newer": { + "$ref": "../examples/short-url-not-found-v3.json" + }, + "Short URL not found previous to API v3": { + "$ref": "../examples/short-url-not-found-v2.json" + } + } + } + } + }, + "default": { + "description": "Unexpected error.", + "content": { + "application/problem+json": { + "schema": { + "$ref": "../definitions/Error.json" + } + } + } + } + } } } diff --git a/docs/swagger/paths/{shortCode}.json b/docs/swagger/paths/{shortCode}.json index bbebacbd..464063da 100644 --- a/docs/swagger/paths/{shortCode}.json +++ b/docs/swagger/paths/{shortCode}.json @@ -8,13 +8,7 @@ "description": "Represents a short URL. Tracks the visit and redirects tio the corresponding long URL", "parameters": [ { - "name": "shortCode", - "in": "path", - "description": "The short code to resolve.", - "required": true, - "schema": { - "type": "string" - } + "$ref": "../parameters/shortCode.json" } ], "responses": { diff --git a/docs/swagger/paths/{shortCode}_qr-code.json b/docs/swagger/paths/{shortCode}_qr-code.json index dd5c8b8a..ca66a079 100644 --- a/docs/swagger/paths/{shortCode}_qr-code.json +++ b/docs/swagger/paths/{shortCode}_qr-code.json @@ -8,13 +8,7 @@ "description": "Generates a QR code image pointing to a short URL.
Since this is not an API endpoint but an image one, when an invalid value is provided for any of the query params, they will fall to their default values instead of throwing an error.", "parameters": [ { - "name": "shortCode", - "in": "path", - "description": "The short code to resolve.", - "required": true, - "schema": { - "type": "string" - } + "$ref": "../parameters/shortCode.json" }, { "name": "size", diff --git a/docs/swagger/paths/{shortCode}_track.json b/docs/swagger/paths/{shortCode}_track.json index 50f6bc5e..96e32411 100644 --- a/docs/swagger/paths/{shortCode}_track.json +++ b/docs/swagger/paths/{shortCode}_track.json @@ -8,13 +8,7 @@ "description": "Generates a 1px transparent image which can be used to track emails with a short URL", "parameters": [ { - "name": "shortCode", - "in": "path", - "description": "The short code to resolve.", - "required": true, - "schema": { - "type": "string" - } + "$ref": "../parameters/shortCode.json" } ], "responses": { From 6bb8c1b2f54515adcdefc4e32057e8ba1868da5e Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 15 May 2023 09:02:23 +0200 Subject: [PATCH 33/55] Rename CLI Option namespace to Input --- module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php | 4 ++-- module/CLI/src/Command/Visit/AbstractVisitsListCommand.php | 4 ++-- module/CLI/src/{Option => Input}/DateOption.php | 2 +- module/CLI/src/{Option => Input}/EndDateOption.php | 2 +- module/CLI/src/{Option => Input}/StartDateOption.php | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-) rename module/CLI/src/{Option => Input}/DateOption.php (97%) rename module/CLI/src/{Option => Input}/EndDateOption.php (95%) rename module/CLI/src/{Option => Input}/StartDateOption.php (95%) diff --git a/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php b/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php index 9202957b..f31de184 100644 --- a/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php +++ b/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php @@ -4,8 +4,8 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI\Command\ShortUrl; -use Shlinkio\Shlink\CLI\Option\EndDateOption; -use Shlinkio\Shlink\CLI\Option\StartDateOption; +use Shlinkio\Shlink\CLI\Input\EndDateOption; +use Shlinkio\Shlink\CLI\Input\StartDateOption; use Shlinkio\Shlink\CLI\Util\ExitCodes; use Shlinkio\Shlink\CLI\Util\ShlinkTable; use Shlinkio\Shlink\Common\Paginator\Paginator; diff --git a/module/CLI/src/Command/Visit/AbstractVisitsListCommand.php b/module/CLI/src/Command/Visit/AbstractVisitsListCommand.php index 402d5ba4..2cd9c0c8 100644 --- a/module/CLI/src/Command/Visit/AbstractVisitsListCommand.php +++ b/module/CLI/src/Command/Visit/AbstractVisitsListCommand.php @@ -4,8 +4,8 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI\Command\Visit; -use Shlinkio\Shlink\CLI\Option\EndDateOption; -use Shlinkio\Shlink\CLI\Option\StartDateOption; +use Shlinkio\Shlink\CLI\Input\EndDateOption; +use Shlinkio\Shlink\CLI\Input\StartDateOption; use Shlinkio\Shlink\CLI\Util\ExitCodes; use Shlinkio\Shlink\CLI\Util\ShlinkTable; use Shlinkio\Shlink\Common\Paginator\Paginator; diff --git a/module/CLI/src/Option/DateOption.php b/module/CLI/src/Input/DateOption.php similarity index 97% rename from module/CLI/src/Option/DateOption.php rename to module/CLI/src/Input/DateOption.php index a863696f..41407d23 100644 --- a/module/CLI/src/Option/DateOption.php +++ b/module/CLI/src/Input/DateOption.php @@ -2,7 +2,7 @@ declare(strict_types=1); -namespace Shlinkio\Shlink\CLI\Option; +namespace Shlinkio\Shlink\CLI\Input; use Cake\Chronos\Chronos; use Symfony\Component\Console\Command\Command; diff --git a/module/CLI/src/Option/EndDateOption.php b/module/CLI/src/Input/EndDateOption.php similarity index 95% rename from module/CLI/src/Option/EndDateOption.php rename to module/CLI/src/Input/EndDateOption.php index 72421981..000a135e 100644 --- a/module/CLI/src/Option/EndDateOption.php +++ b/module/CLI/src/Input/EndDateOption.php @@ -2,7 +2,7 @@ declare(strict_types=1); -namespace Shlinkio\Shlink\CLI\Option; +namespace Shlinkio\Shlink\CLI\Input; use Cake\Chronos\Chronos; use Symfony\Component\Console\Command\Command; diff --git a/module/CLI/src/Option/StartDateOption.php b/module/CLI/src/Input/StartDateOption.php similarity index 95% rename from module/CLI/src/Option/StartDateOption.php rename to module/CLI/src/Input/StartDateOption.php index 2da5aaee..0954e82f 100644 --- a/module/CLI/src/Option/StartDateOption.php +++ b/module/CLI/src/Input/StartDateOption.php @@ -2,7 +2,7 @@ declare(strict_types=1); -namespace Shlinkio\Shlink\CLI\Option; +namespace Shlinkio\Shlink\CLI\Input; use Cake\Chronos\Chronos; use Symfony\Component\Console\Command\Command; From 02a8ef7dd9b8718f76bdc7ba3405d4bdf6a1bc1b Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 15 May 2023 09:43:05 +0200 Subject: [PATCH 34/55] Create DeleteShortUrlVisitsCommand --- module/CLI/config/cli.config.php | 1 + module/CLI/config/dependencies.config.php | 2 + .../CLI/src/Command/Api/DisableKeyCommand.php | 6 +- .../src/Command/Api/GenerateKeyCommand.php | 4 +- .../CLI/src/Command/Api/ListKeysCommand.php | 4 +- .../src/Command/Db/CreateDatabaseCommand.php | 6 +- .../src/Command/Db/MigrateDatabaseCommand.php | 4 +- .../Command/Domain/DomainRedirectsCommand.php | 4 +- .../src/Command/Domain/ListDomainsCommand.php | 4 +- .../ShortUrl/CreateShortUrlCommand.php | 8 +-- .../ShortUrl/DeleteShortUrlCommand.php | 8 +-- .../ShortUrl/DeleteShortUrlVisitsCommand.php | 72 +++++++++++++++++++ .../Command/ShortUrl/ListShortUrlsCommand.php | 4 +- .../Command/ShortUrl/ResolveUrlCommand.php | 6 +- .../CLI/src/Command/Tag/DeleteTagsCommand.php | 6 +- .../CLI/src/Command/Tag/ListTagsCommand.php | 4 +- .../CLI/src/Command/Tag/RenameTagCommand.php | 6 +- .../Command/Util/AbstractLockedCommand.php | 4 +- .../Visit/AbstractVisitsListCommand.php | 4 +- .../Visit/DownloadGeoLiteDbCommand.php | 6 +- .../src/Command/Visit/LocateVisitsCommand.php | 8 +-- .../src/Util/{ExitCodes.php => ExitCode.php} | 2 +- .../test-cli/Command/CreateShortUrlTest.php | 4 +- .../test-cli/Command/GenerateApiKeyTest.php | 4 +- .../CLI/test-cli/Command/ListApiKeysTest.php | 4 +- .../Command/Domain/ListDomainsCommandTest.php | 4 +- .../ShortUrl/CreateShortUrlCommandTest.php | 12 ++-- .../Visit/DownloadGeoLiteDbCommandTest.php | 8 +-- .../Command/Visit/LocateVisitsCommandTest.php | 16 ++--- .../src/ShortUrl/Model/ShortUrlIdentifier.php | 11 +++ .../src/ShortUrl/ShortUrlVisitsDeleter.php | 2 +- .../ShortUrlVisitsDeleterInterface.php | 2 +- .../Repository/VisitDeleterRepositoryTest.php | 1 - .../test-api/Action/CreateShortUrlTest.php | 21 ------ 34 files changed, 163 insertions(+), 99 deletions(-) create mode 100644 module/CLI/src/Command/ShortUrl/DeleteShortUrlVisitsCommand.php rename module/CLI/src/Util/{ExitCodes.php => ExitCode.php} (89%) diff --git a/module/CLI/config/cli.config.php b/module/CLI/config/cli.config.php index 7629d855..012c6800 100644 --- a/module/CLI/config/cli.config.php +++ b/module/CLI/config/cli.config.php @@ -13,6 +13,7 @@ return [ Command\ShortUrl\ListShortUrlsCommand::NAME => Command\ShortUrl\ListShortUrlsCommand::class, Command\ShortUrl\GetShortUrlVisitsCommand::NAME => Command\ShortUrl\GetShortUrlVisitsCommand::class, Command\ShortUrl\DeleteShortUrlCommand::NAME => Command\ShortUrl\DeleteShortUrlCommand::class, + Command\ShortUrl\DeleteShortUrlVisitsCommand::NAME => Command\ShortUrl\DeleteShortUrlVisitsCommand::class, Command\Visit\LocateVisitsCommand::NAME => Command\Visit\LocateVisitsCommand::class, Command\Visit\DownloadGeoLiteDbCommand::NAME => Command\Visit\DownloadGeoLiteDbCommand::class, diff --git a/module/CLI/config/dependencies.config.php b/module/CLI/config/dependencies.config.php index e5176f42..384df91d 100644 --- a/module/CLI/config/dependencies.config.php +++ b/module/CLI/config/dependencies.config.php @@ -42,6 +42,7 @@ return [ Command\ShortUrl\ListShortUrlsCommand::class => ConfigAbstractFactory::class, Command\ShortUrl\GetShortUrlVisitsCommand::class => ConfigAbstractFactory::class, Command\ShortUrl\DeleteShortUrlCommand::class => ConfigAbstractFactory::class, + Command\ShortUrl\DeleteShortUrlVisitsCommand::class => ConfigAbstractFactory::class, Command\Visit\DownloadGeoLiteDbCommand::class => ConfigAbstractFactory::class, Command\Visit\LocateVisitsCommand::class => ConfigAbstractFactory::class, @@ -88,6 +89,7 @@ return [ ], Command\ShortUrl\GetShortUrlVisitsCommand::class => [Visit\VisitsStatsHelper::class], Command\ShortUrl\DeleteShortUrlCommand::class => [ShortUrl\DeleteShortUrlService::class], + Command\ShortUrl\DeleteShortUrlVisitsCommand::class => [ShortUrl\ShortUrlVisitsDeleter::class], Command\Visit\DownloadGeoLiteDbCommand::class => [GeoLite\GeolocationDbUpdater::class], Command\Visit\LocateVisitsCommand::class => [ diff --git a/module/CLI/src/Command/Api/DisableKeyCommand.php b/module/CLI/src/Command/Api/DisableKeyCommand.php index 7296632a..4844121e 100644 --- a/module/CLI/src/Command/Api/DisableKeyCommand.php +++ b/module/CLI/src/Command/Api/DisableKeyCommand.php @@ -4,7 +4,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI\Command\Api; -use Shlinkio\Shlink\CLI\Util\ExitCodes; +use Shlinkio\Shlink\CLI\Util\ExitCode; use Shlinkio\Shlink\Common\Exception\InvalidArgumentException; use Shlinkio\Shlink\Rest\Service\ApiKeyServiceInterface; use Symfony\Component\Console\Command\Command; @@ -39,10 +39,10 @@ class DisableKeyCommand extends Command try { $this->apiKeyService->disable($apiKey); $io->success(sprintf('API key "%s" properly disabled', $apiKey)); - return ExitCodes::EXIT_SUCCESS; + return ExitCode::EXIT_SUCCESS; } catch (InvalidArgumentException $e) { $io->error($e->getMessage()); - return ExitCodes::EXIT_FAILURE; + return ExitCode::EXIT_FAILURE; } } } diff --git a/module/CLI/src/Command/Api/GenerateKeyCommand.php b/module/CLI/src/Command/Api/GenerateKeyCommand.php index c89c4fbf..c2d6cf10 100644 --- a/module/CLI/src/Command/Api/GenerateKeyCommand.php +++ b/module/CLI/src/Command/Api/GenerateKeyCommand.php @@ -6,7 +6,7 @@ namespace Shlinkio\Shlink\CLI\Command\Api; use Cake\Chronos\Chronos; use Shlinkio\Shlink\CLI\ApiKey\RoleResolverInterface; -use Shlinkio\Shlink\CLI\Util\ExitCodes; +use Shlinkio\Shlink\CLI\Util\ExitCode; use Shlinkio\Shlink\CLI\Util\ShlinkTable; use Shlinkio\Shlink\Rest\ApiKey\Role; use Shlinkio\Shlink\Rest\Entity\ApiKey; @@ -109,6 +109,6 @@ class GenerateKeyCommand extends Command ); } - return ExitCodes::EXIT_SUCCESS; + return ExitCode::EXIT_SUCCESS; } } diff --git a/module/CLI/src/Command/Api/ListKeysCommand.php b/module/CLI/src/Command/Api/ListKeysCommand.php index c7e31819..87b239b7 100644 --- a/module/CLI/src/Command/Api/ListKeysCommand.php +++ b/module/CLI/src/Command/Api/ListKeysCommand.php @@ -4,7 +4,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI\Command\Api; -use Shlinkio\Shlink\CLI\Util\ExitCodes; +use Shlinkio\Shlink\CLI\Util\ExitCode; use Shlinkio\Shlink\CLI\Util\ShlinkTable; use Shlinkio\Shlink\Rest\ApiKey\Role; use Shlinkio\Shlink\Rest\Entity\ApiKey; @@ -77,7 +77,7 @@ class ListKeysCommand extends Command 'Roles', ]), $rows); - return ExitCodes::EXIT_SUCCESS; + return ExitCode::EXIT_SUCCESS; } private function determineMessagePattern(ApiKey $apiKey): string diff --git a/module/CLI/src/Command/Db/CreateDatabaseCommand.php b/module/CLI/src/Command/Db/CreateDatabaseCommand.php index 95b08da2..f6df9b04 100644 --- a/module/CLI/src/Command/Db/CreateDatabaseCommand.php +++ b/module/CLI/src/Command/Db/CreateDatabaseCommand.php @@ -8,7 +8,7 @@ use Doctrine\DBAL\Connection; use Doctrine\DBAL\Platforms\SqlitePlatform; use Doctrine\ORM\EntityManagerInterface; use Doctrine\ORM\Mapping\ClassMetadata; -use Shlinkio\Shlink\CLI\Util\ExitCodes; +use Shlinkio\Shlink\CLI\Util\ExitCode; use Shlinkio\Shlink\CLI\Util\ProcessRunnerInterface; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; @@ -57,7 +57,7 @@ class CreateDatabaseCommand extends AbstractDatabaseCommand if ($this->schemaExists()) { $io->success('Database already exists. Run "db:migrate" command to make sure it is up to date.'); - return ExitCodes::EXIT_SUCCESS; + return ExitCode::EXIT_SUCCESS; } // Create database @@ -65,7 +65,7 @@ class CreateDatabaseCommand extends AbstractDatabaseCommand $this->runPhpCommand($output, [self::DOCTRINE_SCRIPT, self::DOCTRINE_CREATE_SCHEMA_COMMAND]); $io->success('Database properly created!'); - return ExitCodes::EXIT_SUCCESS; + return ExitCode::EXIT_SUCCESS; } private function checkDbExists(): void diff --git a/module/CLI/src/Command/Db/MigrateDatabaseCommand.php b/module/CLI/src/Command/Db/MigrateDatabaseCommand.php index 379e57e0..a912cf24 100644 --- a/module/CLI/src/Command/Db/MigrateDatabaseCommand.php +++ b/module/CLI/src/Command/Db/MigrateDatabaseCommand.php @@ -4,7 +4,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI\Command\Db; -use Shlinkio\Shlink\CLI\Util\ExitCodes; +use Shlinkio\Shlink\CLI\Util\ExitCode; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; use Symfony\Component\Console\Style\SymfonyStyle; @@ -31,6 +31,6 @@ class MigrateDatabaseCommand extends AbstractDatabaseCommand $this->runPhpCommand($output, [self::DOCTRINE_MIGRATIONS_SCRIPT, self::DOCTRINE_MIGRATE_COMMAND]); $io->success('Database properly migrated!'); - return ExitCodes::EXIT_SUCCESS; + return ExitCode::EXIT_SUCCESS; } } diff --git a/module/CLI/src/Command/Domain/DomainRedirectsCommand.php b/module/CLI/src/Command/Domain/DomainRedirectsCommand.php index c546fd5b..4a3f8062 100644 --- a/module/CLI/src/Command/Domain/DomainRedirectsCommand.php +++ b/module/CLI/src/Command/Domain/DomainRedirectsCommand.php @@ -4,7 +4,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI\Command\Domain; -use Shlinkio\Shlink\CLI\Util\ExitCodes; +use Shlinkio\Shlink\CLI\Util\ExitCode; use Shlinkio\Shlink\Core\Config\NotFoundRedirects; use Shlinkio\Shlink\Core\Domain\DomainServiceInterface; use Shlinkio\Shlink\Core\Domain\Model\DomainItem; @@ -109,6 +109,6 @@ class DomainRedirectsCommand extends Command $io->success(sprintf('"Not found" redirects properly set for "%s"', $domainAuthority)); - return ExitCodes::EXIT_SUCCESS; + return ExitCode::EXIT_SUCCESS; } } diff --git a/module/CLI/src/Command/Domain/ListDomainsCommand.php b/module/CLI/src/Command/Domain/ListDomainsCommand.php index 8f2ee22c..11a0f5b9 100644 --- a/module/CLI/src/Command/Domain/ListDomainsCommand.php +++ b/module/CLI/src/Command/Domain/ListDomainsCommand.php @@ -4,7 +4,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI\Command\Domain; -use Shlinkio\Shlink\CLI\Util\ExitCodes; +use Shlinkio\Shlink\CLI\Util\ExitCode; use Shlinkio\Shlink\CLI\Util\ShlinkTable; use Shlinkio\Shlink\Core\Config\NotFoundRedirectConfigInterface; use Shlinkio\Shlink\Core\Domain\DomainServiceInterface; @@ -59,7 +59,7 @@ class ListDomainsCommand extends Command }), ); - return ExitCodes::EXIT_SUCCESS; + return ExitCode::EXIT_SUCCESS; } private function notFoundRedirectsToString(NotFoundRedirectConfigInterface $config): string diff --git a/module/CLI/src/Command/ShortUrl/CreateShortUrlCommand.php b/module/CLI/src/Command/ShortUrl/CreateShortUrlCommand.php index a998a677..f55f247d 100644 --- a/module/CLI/src/Command/ShortUrl/CreateShortUrlCommand.php +++ b/module/CLI/src/Command/ShortUrl/CreateShortUrlCommand.php @@ -4,7 +4,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI\Command\ShortUrl; -use Shlinkio\Shlink\CLI\Util\ExitCodes; +use Shlinkio\Shlink\CLI\Util\ExitCode; use Shlinkio\Shlink\Core\Exception\InvalidUrlException; use Shlinkio\Shlink\Core\Exception\NonUniqueSlugException; use Shlinkio\Shlink\Core\Options\UrlShortenerOptions; @@ -141,7 +141,7 @@ class CreateShortUrlCommand extends Command $longUrl = $input->getArgument('longUrl'); if (empty($longUrl)) { $io->error('A URL was not provided!'); - return ExitCodes::EXIT_FAILURE; + return ExitCode::EXIT_FAILURE; } $explodeWithComma = curry(explode(...))(','); @@ -176,10 +176,10 @@ class CreateShortUrlCommand extends Command sprintf('Processed long URL: %s', $longUrl), sprintf('Generated short URL: %s', $this->stringifier->stringify($result->shortUrl)), ]); - return ExitCodes::EXIT_SUCCESS; + return ExitCode::EXIT_SUCCESS; } catch (InvalidUrlException | NonUniqueSlugException $e) { $io->error($e->getMessage()); - return ExitCodes::EXIT_FAILURE; + return ExitCode::EXIT_FAILURE; } } diff --git a/module/CLI/src/Command/ShortUrl/DeleteShortUrlCommand.php b/module/CLI/src/Command/ShortUrl/DeleteShortUrlCommand.php index 1db5b1f6..11cfa270 100644 --- a/module/CLI/src/Command/ShortUrl/DeleteShortUrlCommand.php +++ b/module/CLI/src/Command/ShortUrl/DeleteShortUrlCommand.php @@ -4,7 +4,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI\Command\ShortUrl; -use Shlinkio\Shlink\CLI\Util\ExitCodes; +use Shlinkio\Shlink\CLI\Util\ExitCode; use Shlinkio\Shlink\Core\Exception; use Shlinkio\Shlink\Core\ShortUrl\DeleteShortUrlServiceInterface; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlIdentifier; @@ -55,10 +55,10 @@ class DeleteShortUrlCommand extends Command try { $this->runDelete($io, $identifier, $ignoreThreshold); - return ExitCodes::EXIT_SUCCESS; + return ExitCode::EXIT_SUCCESS; } catch (Exception\ShortUrlNotFoundException $e) { $io->error($e->getMessage()); - return ExitCodes::EXIT_FAILURE; + return ExitCode::EXIT_FAILURE; } catch (Exception\DeleteShortUrlException $e) { return $this->retry($io, $identifier, $e->getMessage()); } @@ -75,7 +75,7 @@ class DeleteShortUrlCommand extends Command $io->warning('Short URL was not deleted.'); } - return $forceDelete ? ExitCodes::EXIT_SUCCESS : ExitCodes::EXIT_WARNING; + return $forceDelete ? ExitCode::EXIT_SUCCESS : ExitCode::EXIT_WARNING; } private function runDelete(SymfonyStyle $io, ShortUrlIdentifier $identifier, bool $ignoreThreshold): void diff --git a/module/CLI/src/Command/ShortUrl/DeleteShortUrlVisitsCommand.php b/module/CLI/src/Command/ShortUrl/DeleteShortUrlVisitsCommand.php new file mode 100644 index 00000000..fa7a8ee6 --- /dev/null +++ b/module/CLI/src/Command/ShortUrl/DeleteShortUrlVisitsCommand.php @@ -0,0 +1,72 @@ +setName(self::NAME) + ->setDescription('Deletes visits from a short URL') + ->addArgument( + 'shortCode', + InputArgument::REQUIRED, + 'The short code for the short URL which visits will be deleted', + ) + ->addOption( + 'domain', + 'd', + InputOption::VALUE_REQUIRED, + 'The domain if the short code does not belong to the default one', + ); + } + + protected function execute(InputInterface $input, OutputInterface $output): ?int + { + $identifier = ShortUrlIdentifier::fromCli($input); + $io = new SymfonyStyle($input, $output); + if (! $this->confirm($io)) { + $io->info('Operation aborted'); + return ExitCode::EXIT_SUCCESS; + } + + try { + $result = $this->deleter->deleteShortUrlVisits($identifier); + $io->success(sprintf('Successfully deleted %s visits', $result->affectedItems)); + + return ExitCode::EXIT_SUCCESS; + } catch (ShortUrlNotFoundException) { + $io->warning(sprintf('Short URL not found for "%s"', $identifier->__toString())); + return ExitCode::EXIT_WARNING; + } + } + + private function confirm(SymfonyStyle $io): bool + { + $io->warning('You are about to delete all visits for a short URL. This operation cannot be undone.'); + return $io->confirm('Continue deleting visits?', false); + } +} diff --git a/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php b/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php index f31de184..14ea1851 100644 --- a/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php +++ b/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php @@ -6,7 +6,7 @@ namespace Shlinkio\Shlink\CLI\Command\ShortUrl; use Shlinkio\Shlink\CLI\Input\EndDateOption; use Shlinkio\Shlink\CLI\Input\StartDateOption; -use Shlinkio\Shlink\CLI\Util\ExitCodes; +use Shlinkio\Shlink\CLI\Util\ExitCode; use Shlinkio\Shlink\CLI\Util\ShlinkTable; use Shlinkio\Shlink\Common\Paginator\Paginator; use Shlinkio\Shlink\Common\Paginator\Util\PagerfantaUtilsTrait; @@ -173,7 +173,7 @@ class ListShortUrlsCommand extends Command $io->newLine(); $io->success('Short URLs properly listed'); - return ExitCodes::EXIT_SUCCESS; + return ExitCode::EXIT_SUCCESS; } private function renderPage( diff --git a/module/CLI/src/Command/ShortUrl/ResolveUrlCommand.php b/module/CLI/src/Command/ShortUrl/ResolveUrlCommand.php index 8d54edd2..aec0a843 100644 --- a/module/CLI/src/Command/ShortUrl/ResolveUrlCommand.php +++ b/module/CLI/src/Command/ShortUrl/ResolveUrlCommand.php @@ -4,7 +4,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI\Command\ShortUrl; -use Shlinkio\Shlink\CLI\Util\ExitCodes; +use Shlinkio\Shlink\CLI\Util\ExitCode; use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\ShortUrl\ShortUrlResolverInterface; @@ -56,10 +56,10 @@ class ResolveUrlCommand extends Command try { $url = $this->urlResolver->resolveShortUrl(ShortUrlIdentifier::fromCli($input)); $output->writeln(sprintf('Long URL: %s', $url->getLongUrl())); - return ExitCodes::EXIT_SUCCESS; + return ExitCode::EXIT_SUCCESS; } catch (ShortUrlNotFoundException $e) { $io->error($e->getMessage()); - return ExitCodes::EXIT_FAILURE; + return ExitCode::EXIT_FAILURE; } } } diff --git a/module/CLI/src/Command/Tag/DeleteTagsCommand.php b/module/CLI/src/Command/Tag/DeleteTagsCommand.php index 5a4f81ac..151c5892 100644 --- a/module/CLI/src/Command/Tag/DeleteTagsCommand.php +++ b/module/CLI/src/Command/Tag/DeleteTagsCommand.php @@ -4,7 +4,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI\Command\Tag; -use Shlinkio\Shlink\CLI\Util\ExitCodes; +use Shlinkio\Shlink\CLI\Util\ExitCode; use Shlinkio\Shlink\Core\Tag\TagServiceInterface; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputInterface; @@ -41,11 +41,11 @@ class DeleteTagsCommand extends Command if (empty($tagNames)) { $io->warning('You have to provide at least one tag name'); - return ExitCodes::EXIT_WARNING; + return ExitCode::EXIT_WARNING; } $this->tagService->deleteTags($tagNames); $io->success('Tags properly deleted'); - return ExitCodes::EXIT_SUCCESS; + return ExitCode::EXIT_SUCCESS; } } diff --git a/module/CLI/src/Command/Tag/ListTagsCommand.php b/module/CLI/src/Command/Tag/ListTagsCommand.php index 02116d79..41ca9b60 100644 --- a/module/CLI/src/Command/Tag/ListTagsCommand.php +++ b/module/CLI/src/Command/Tag/ListTagsCommand.php @@ -4,7 +4,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI\Command\Tag; -use Shlinkio\Shlink\CLI\Util\ExitCodes; +use Shlinkio\Shlink\CLI\Util\ExitCode; use Shlinkio\Shlink\CLI\Util\ShlinkTable; use Shlinkio\Shlink\Core\Tag\Model\TagInfo; use Shlinkio\Shlink\Core\Tag\Model\TagsParams; @@ -34,7 +34,7 @@ class ListTagsCommand extends Command protected function execute(InputInterface $input, OutputInterface $output): ?int { ShlinkTable::default($output)->render(['Name', 'URLs amount', 'Visits amount'], $this->getTagsRows()); - return ExitCodes::EXIT_SUCCESS; + return ExitCode::EXIT_SUCCESS; } private function getTagsRows(): array diff --git a/module/CLI/src/Command/Tag/RenameTagCommand.php b/module/CLI/src/Command/Tag/RenameTagCommand.php index 85377a18..1da3b983 100644 --- a/module/CLI/src/Command/Tag/RenameTagCommand.php +++ b/module/CLI/src/Command/Tag/RenameTagCommand.php @@ -4,7 +4,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI\Command\Tag; -use Shlinkio\Shlink\CLI\Util\ExitCodes; +use Shlinkio\Shlink\CLI\Util\ExitCode; use Shlinkio\Shlink\Core\Exception\TagConflictException; use Shlinkio\Shlink\Core\Exception\TagNotFoundException; use Shlinkio\Shlink\Core\Tag\Model\TagRenaming; @@ -42,10 +42,10 @@ class RenameTagCommand extends Command try { $this->tagService->renameTag(TagRenaming::fromNames($oldName, $newName)); $io->success('Tag properly renamed.'); - return ExitCodes::EXIT_SUCCESS; + return ExitCode::EXIT_SUCCESS; } catch (TagNotFoundException | TagConflictException $e) { $io->error($e->getMessage()); - return ExitCodes::EXIT_FAILURE; + return ExitCode::EXIT_FAILURE; } } } diff --git a/module/CLI/src/Command/Util/AbstractLockedCommand.php b/module/CLI/src/Command/Util/AbstractLockedCommand.php index d1e45fd8..ae930496 100644 --- a/module/CLI/src/Command/Util/AbstractLockedCommand.php +++ b/module/CLI/src/Command/Util/AbstractLockedCommand.php @@ -4,7 +4,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI\Command\Util; -use Shlinkio\Shlink\CLI\Util\ExitCodes; +use Shlinkio\Shlink\CLI\Util\ExitCode; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; @@ -28,7 +28,7 @@ abstract class AbstractLockedCommand extends Command $output->writeln( sprintf('Command "%s" is already in progress. Skipping.', $lockConfig->lockName), ); - return ExitCodes::EXIT_WARNING; + return ExitCode::EXIT_WARNING; } try { diff --git a/module/CLI/src/Command/Visit/AbstractVisitsListCommand.php b/module/CLI/src/Command/Visit/AbstractVisitsListCommand.php index 2cd9c0c8..ba518656 100644 --- a/module/CLI/src/Command/Visit/AbstractVisitsListCommand.php +++ b/module/CLI/src/Command/Visit/AbstractVisitsListCommand.php @@ -6,7 +6,7 @@ namespace Shlinkio\Shlink\CLI\Command\Visit; use Shlinkio\Shlink\CLI\Input\EndDateOption; use Shlinkio\Shlink\CLI\Input\StartDateOption; -use Shlinkio\Shlink\CLI\Util\ExitCodes; +use Shlinkio\Shlink\CLI\Util\ExitCode; use Shlinkio\Shlink\CLI\Util\ShlinkTable; use Shlinkio\Shlink\Common\Paginator\Paginator; use Shlinkio\Shlink\Common\Util\DateRange; @@ -43,7 +43,7 @@ abstract class AbstractVisitsListCommand extends Command ShlinkTable::default($output)->render($headers, $rows); - return ExitCodes::EXIT_SUCCESS; + return ExitCode::EXIT_SUCCESS; } private function resolveRowsAndHeaders(Paginator $paginator): array diff --git a/module/CLI/src/Command/Visit/DownloadGeoLiteDbCommand.php b/module/CLI/src/Command/Visit/DownloadGeoLiteDbCommand.php index c4384d33..23600530 100644 --- a/module/CLI/src/Command/Visit/DownloadGeoLiteDbCommand.php +++ b/module/CLI/src/Command/Visit/DownloadGeoLiteDbCommand.php @@ -6,7 +6,7 @@ namespace Shlinkio\Shlink\CLI\Command\Visit; use Shlinkio\Shlink\CLI\Exception\GeolocationDbUpdateFailedException; use Shlinkio\Shlink\CLI\GeoLite\GeolocationDbUpdaterInterface; -use Shlinkio\Shlink\CLI\Util\ExitCodes; +use Shlinkio\Shlink\CLI\Util\ExitCode; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Helper\ProgressBar; use Symfony\Component\Console\Input\InputInterface; @@ -56,7 +56,7 @@ class DownloadGeoLiteDbCommand extends Command $io->success('GeoLite2 db file properly downloaded.'); } - return ExitCodes::EXIT_SUCCESS; + return ExitCode::EXIT_SUCCESS; } catch (GeolocationDbUpdateFailedException $e) { $olderDbExists = $e->olderDbExists(); @@ -72,7 +72,7 @@ class DownloadGeoLiteDbCommand extends Command $this->getApplication()?->renderThrowable($e, $io); } - return $olderDbExists ? ExitCodes::EXIT_WARNING : ExitCodes::EXIT_FAILURE; + return $olderDbExists ? ExitCode::EXIT_WARNING : ExitCode::EXIT_FAILURE; } } } diff --git a/module/CLI/src/Command/Visit/LocateVisitsCommand.php b/module/CLI/src/Command/Visit/LocateVisitsCommand.php index d83c91e0..09e53556 100644 --- a/module/CLI/src/Command/Visit/LocateVisitsCommand.php +++ b/module/CLI/src/Command/Visit/LocateVisitsCommand.php @@ -6,7 +6,7 @@ namespace Shlinkio\Shlink\CLI\Command\Visit; use Shlinkio\Shlink\CLI\Command\Util\AbstractLockedCommand; use Shlinkio\Shlink\CLI\Command\Util\LockedCommandConfig; -use Shlinkio\Shlink\CLI\Util\ExitCodes; +use Shlinkio\Shlink\CLI\Util\ExitCode; use Shlinkio\Shlink\Common\Util\IpAddress; use Shlinkio\Shlink\Core\Exception\IpCannotBeLocatedException; use Shlinkio\Shlink\Core\Visit\Entity\Visit; @@ -116,14 +116,14 @@ class LocateVisitsCommand extends AbstractLockedCommand implements VisitGeolocat } $this->io->success('Finished locating visits'); - return ExitCodes::EXIT_SUCCESS; + return ExitCode::EXIT_SUCCESS; } catch (Throwable $e) { $this->io->error($e->getMessage()); if ($this->io->isVerbose()) { $this->getApplication()?->renderThrowable($e, $this->io); } - return ExitCodes::EXIT_FAILURE; + return ExitCode::EXIT_FAILURE; } } @@ -171,7 +171,7 @@ class LocateVisitsCommand extends AbstractLockedCommand implements VisitGeolocat $downloadDbCommand = $cliApp->find(DownloadGeoLiteDbCommand::NAME); $exitCode = $downloadDbCommand->run(new ArrayInput([]), $this->io); - if ($exitCode === ExitCodes::EXIT_FAILURE) { + if ($exitCode === ExitCode::EXIT_FAILURE) { throw new RuntimeException('It is not possible to locate visits without a GeoLite2 db file.'); } } diff --git a/module/CLI/src/Util/ExitCodes.php b/module/CLI/src/Util/ExitCode.php similarity index 89% rename from module/CLI/src/Util/ExitCodes.php rename to module/CLI/src/Util/ExitCode.php index d915796a..128b9f52 100644 --- a/module/CLI/src/Util/ExitCodes.php +++ b/module/CLI/src/Util/ExitCode.php @@ -4,7 +4,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI\Util; -final class ExitCodes +final class ExitCode { public const EXIT_SUCCESS = 0; public const EXIT_FAILURE = -1; diff --git a/module/CLI/test-cli/Command/CreateShortUrlTest.php b/module/CLI/test-cli/Command/CreateShortUrlTest.php index d4d8a583..c2e96611 100644 --- a/module/CLI/test-cli/Command/CreateShortUrlTest.php +++ b/module/CLI/test-cli/Command/CreateShortUrlTest.php @@ -7,7 +7,7 @@ namespace ShlinkioCliTest\Shlink\CLI\Command; use PHPUnit\Framework\Attributes\Test; use Shlinkio\Shlink\CLI\Command\ShortUrl\CreateShortUrlCommand; use Shlinkio\Shlink\CLI\Command\ShortUrl\ListShortUrlsCommand; -use Shlinkio\Shlink\CLI\Util\ExitCodes; +use Shlinkio\Shlink\CLI\Util\ExitCode; use Shlinkio\Shlink\TestUtils\CliTest\CliTestCase; class CreateShortUrlTest extends CliTestCase @@ -22,7 +22,7 @@ class CreateShortUrlTest extends CliTestCase [CreateShortUrlCommand::NAME, 'https://example.com', '--domain', $defaultDomain, '--custom-slug', $slug], ); - self::assertEquals(ExitCodes::EXIT_SUCCESS, $exitCode); + self::assertEquals(ExitCode::EXIT_SUCCESS, $exitCode); self::assertStringContainsString('Generated short URL: http://' . $defaultDomain . '/' . $slug, $output); [$listOutput] = $this->exec([ListShortUrlsCommand::NAME, '--show-domain', '--search-term', $slug]); diff --git a/module/CLI/test-cli/Command/GenerateApiKeyTest.php b/module/CLI/test-cli/Command/GenerateApiKeyTest.php index c98dc237..7d90c336 100644 --- a/module/CLI/test-cli/Command/GenerateApiKeyTest.php +++ b/module/CLI/test-cli/Command/GenerateApiKeyTest.php @@ -6,7 +6,7 @@ namespace ShlinkioCliTest\Shlink\CLI\Command; use PHPUnit\Framework\Attributes\Test; use Shlinkio\Shlink\CLI\Command\Api\GenerateKeyCommand; -use Shlinkio\Shlink\CLI\Util\ExitCodes; +use Shlinkio\Shlink\CLI\Util\ExitCode; use Shlinkio\Shlink\TestUtils\CliTest\CliTestCase; class GenerateApiKeyTest extends CliTestCase @@ -17,6 +17,6 @@ class GenerateApiKeyTest extends CliTestCase [$output, $exitCode] = $this->exec([GenerateKeyCommand::NAME]); self::assertStringContainsString('[OK] Generated API key', $output); - self::assertEquals(ExitCodes::EXIT_SUCCESS, $exitCode); + self::assertEquals(ExitCode::EXIT_SUCCESS, $exitCode); } } diff --git a/module/CLI/test-cli/Command/ListApiKeysTest.php b/module/CLI/test-cli/Command/ListApiKeysTest.php index 80a1134d..f8781d54 100644 --- a/module/CLI/test-cli/Command/ListApiKeysTest.php +++ b/module/CLI/test-cli/Command/ListApiKeysTest.php @@ -8,7 +8,7 @@ use Cake\Chronos\Chronos; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Test; use Shlinkio\Shlink\CLI\Command\Api\ListKeysCommand; -use Shlinkio\Shlink\CLI\Util\ExitCodes; +use Shlinkio\Shlink\CLI\Util\ExitCode; use Shlinkio\Shlink\TestUtils\CliTest\CliTestCase; class ListApiKeysTest extends CliTestCase @@ -19,7 +19,7 @@ class ListApiKeysTest extends CliTestCase [$output, $exitCode] = $this->exec([ListKeysCommand::NAME, ...$flags]); self::assertEquals($expectedOutput, $output); - self::assertEquals(ExitCodes::EXIT_SUCCESS, $exitCode); + self::assertEquals(ExitCode::EXIT_SUCCESS, $exitCode); } public static function provideFlags(): iterable diff --git a/module/CLI/test/Command/Domain/ListDomainsCommandTest.php b/module/CLI/test/Command/Domain/ListDomainsCommandTest.php index ad31d86d..05cc95eb 100644 --- a/module/CLI/test/Command/Domain/ListDomainsCommandTest.php +++ b/module/CLI/test/Command/Domain/ListDomainsCommandTest.php @@ -9,7 +9,7 @@ use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Shlinkio\Shlink\CLI\Command\Domain\ListDomainsCommand; -use Shlinkio\Shlink\CLI\Util\ExitCodes; +use Shlinkio\Shlink\CLI\Util\ExitCode; use Shlinkio\Shlink\Core\Config\NotFoundRedirects; use Shlinkio\Shlink\Core\Domain\DomainServiceInterface; use Shlinkio\Shlink\Core\Domain\Entity\Domain; @@ -53,7 +53,7 @@ class ListDomainsCommandTest extends TestCase $this->commandTester->execute($input); self::assertEquals($expectedOutput, $this->commandTester->getDisplay()); - self::assertEquals(ExitCodes::EXIT_SUCCESS, $this->commandTester->getStatusCode()); + self::assertEquals(ExitCode::EXIT_SUCCESS, $this->commandTester->getStatusCode()); } public static function provideInputsAndOutputs(): iterable diff --git a/module/CLI/test/Command/ShortUrl/CreateShortUrlCommandTest.php b/module/CLI/test/Command/ShortUrl/CreateShortUrlCommandTest.php index 60482138..46063485 100644 --- a/module/CLI/test/Command/ShortUrl/CreateShortUrlCommandTest.php +++ b/module/CLI/test/Command/ShortUrl/CreateShortUrlCommandTest.php @@ -11,7 +11,7 @@ use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Shlinkio\Shlink\CLI\Command\ShortUrl\CreateShortUrlCommand; -use Shlinkio\Shlink\CLI\Util\ExitCodes; +use Shlinkio\Shlink\CLI\Util\ExitCode; use Shlinkio\Shlink\Core\Exception\InvalidUrlException; use Shlinkio\Shlink\Core\Exception\NonUniqueSlugException; use Shlinkio\Shlink\Core\Options\UrlShortenerOptions; @@ -65,7 +65,7 @@ class CreateShortUrlCommandTest extends TestCase ], ['verbosity' => OutputInterface::VERBOSITY_VERBOSE]); $output = $this->commandTester->getDisplay(); - self::assertEquals(ExitCodes::EXIT_SUCCESS, $this->commandTester->getStatusCode()); + self::assertEquals(ExitCode::EXIT_SUCCESS, $this->commandTester->getStatusCode()); self::assertStringContainsString('stringified_short_url', $output); self::assertStringNotContainsString('but the real-time updates cannot', $output); } @@ -82,7 +82,7 @@ class CreateShortUrlCommandTest extends TestCase $this->commandTester->execute(['longUrl' => $url]); $output = $this->commandTester->getDisplay(); - self::assertEquals(ExitCodes::EXIT_FAILURE, $this->commandTester->getStatusCode()); + self::assertEquals(ExitCode::EXIT_FAILURE, $this->commandTester->getStatusCode()); self::assertStringContainsString('Provided URL http://domain.com/invalid is invalid.', $output); } @@ -97,7 +97,7 @@ class CreateShortUrlCommandTest extends TestCase $this->commandTester->execute(['longUrl' => 'http://domain.com/invalid', '--custom-slug' => 'my-slug']); $output = $this->commandTester->getDisplay(); - self::assertEquals(ExitCodes::EXIT_FAILURE, $this->commandTester->getStatusCode()); + self::assertEquals(ExitCode::EXIT_FAILURE, $this->commandTester->getStatusCode()); self::assertStringContainsString('Provided slug "my-slug" is already in use', $output); } @@ -121,7 +121,7 @@ class CreateShortUrlCommandTest extends TestCase ]); $output = $this->commandTester->getDisplay(); - self::assertEquals(ExitCodes::EXIT_SUCCESS, $this->commandTester->getStatusCode()); + self::assertEquals(ExitCode::EXIT_SUCCESS, $this->commandTester->getStatusCode()); self::assertStringContainsString('stringified_short_url', $output); } @@ -139,7 +139,7 @@ class CreateShortUrlCommandTest extends TestCase $input['longUrl'] = 'http://domain.com/foo/bar'; $this->commandTester->execute($input); - self::assertEquals(ExitCodes::EXIT_SUCCESS, $this->commandTester->getStatusCode()); + self::assertEquals(ExitCode::EXIT_SUCCESS, $this->commandTester->getStatusCode()); } public static function provideDomains(): iterable diff --git a/module/CLI/test/Command/Visit/DownloadGeoLiteDbCommandTest.php b/module/CLI/test/Command/Visit/DownloadGeoLiteDbCommandTest.php index 7f2cb3ac..7e904caa 100644 --- a/module/CLI/test/Command/Visit/DownloadGeoLiteDbCommandTest.php +++ b/module/CLI/test/Command/Visit/DownloadGeoLiteDbCommandTest.php @@ -12,7 +12,7 @@ use Shlinkio\Shlink\CLI\Command\Visit\DownloadGeoLiteDbCommand; use Shlinkio\Shlink\CLI\Exception\GeolocationDbUpdateFailedException; use Shlinkio\Shlink\CLI\GeoLite\GeolocationDbUpdaterInterface; use Shlinkio\Shlink\CLI\GeoLite\GeolocationResult; -use Shlinkio\Shlink\CLI\Util\ExitCodes; +use Shlinkio\Shlink\CLI\Util\ExitCode; use ShlinkioTest\Shlink\CLI\CliTestUtilsTrait; use Symfony\Component\Console\Tester\CommandTester; @@ -65,12 +65,12 @@ class DownloadGeoLiteDbCommandTest extends TestCase yield 'existing db' => [ true, '[WARNING] GeoLite2 db file update failed. Visits will continue to be located', - ExitCodes::EXIT_WARNING, + ExitCode::EXIT_WARNING, ]; yield 'not existing db' => [ false, '[ERROR] GeoLite2 db file download failed. It will not be possible to locate', - ExitCodes::EXIT_FAILURE, + ExitCode::EXIT_FAILURE, ]; } @@ -86,7 +86,7 @@ class DownloadGeoLiteDbCommandTest extends TestCase $exitCode = $this->commandTester->getStatusCode(); self::assertStringContainsString($expectedMessage, $output); - self::assertSame(ExitCodes::EXIT_SUCCESS, $exitCode); + self::assertSame(ExitCode::EXIT_SUCCESS, $exitCode); } public static function provideSuccessParams(): iterable diff --git a/module/CLI/test/Command/Visit/LocateVisitsCommandTest.php b/module/CLI/test/Command/Visit/LocateVisitsCommandTest.php index aa775a24..6ff8c242 100644 --- a/module/CLI/test/Command/Visit/LocateVisitsCommandTest.php +++ b/module/CLI/test/Command/Visit/LocateVisitsCommandTest.php @@ -10,7 +10,7 @@ use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Shlinkio\Shlink\CLI\Command\Visit\DownloadGeoLiteDbCommand; use Shlinkio\Shlink\CLI\Command\Visit\LocateVisitsCommand; -use Shlinkio\Shlink\CLI\Util\ExitCodes; +use Shlinkio\Shlink\CLI\Util\ExitCode; use Shlinkio\Shlink\Core\Exception\IpCannotBeLocatedException; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; use Shlinkio\Shlink\Core\Visit\Entity\Visit; @@ -85,7 +85,7 @@ class LocateVisitsCommandTest extends TestCase $this->visitToLocation->expects( $this->exactly($expectedUnlocatedCalls + $expectedEmptyCalls + $expectedAllCalls), )->method('resolveVisitLocation')->withAnyParameters()->willReturn(Location::emptyInstance()); - $this->downloadDbCommand->method('run')->withAnyParameters()->willReturn(ExitCodes::EXIT_SUCCESS); + $this->downloadDbCommand->method('run')->withAnyParameters()->willReturn(ExitCode::EXIT_SUCCESS); $this->commandTester->setInputs(['y']); $this->commandTester->execute($args); @@ -118,7 +118,7 @@ class LocateVisitsCommandTest extends TestCase ->withAnyParameters() ->willReturnCallback($this->invokeHelperMethods($visit, $location)); $this->visitToLocation->expects($this->once())->method('resolveVisitLocation')->willThrowException($e); - $this->downloadDbCommand->method('run')->withAnyParameters()->willReturn(ExitCodes::EXIT_SUCCESS); + $this->downloadDbCommand->method('run')->withAnyParameters()->willReturn(ExitCode::EXIT_SUCCESS); $this->commandTester->execute([], ['verbosity' => OutputInterface::VERBOSITY_VERBOSE]); @@ -147,7 +147,7 @@ class LocateVisitsCommandTest extends TestCase $this->visitToLocation->expects($this->once())->method('resolveVisitLocation')->willThrowException( IpCannotBeLocatedException::forError(WrongIpException::fromIpAddress('1.2.3.4')), ); - $this->downloadDbCommand->method('run')->withAnyParameters()->willReturn(ExitCodes::EXIT_SUCCESS); + $this->downloadDbCommand->method('run')->withAnyParameters()->willReturn(ExitCode::EXIT_SUCCESS); $this->commandTester->execute([], ['verbosity' => OutputInterface::VERBOSITY_VERBOSE]); @@ -171,7 +171,7 @@ class LocateVisitsCommandTest extends TestCase $this->visitService->expects($this->never())->method('locateUnlocatedVisits'); $this->visitToLocation->expects($this->never())->method('resolveVisitLocation'); - $this->downloadDbCommand->method('run')->withAnyParameters()->willReturn(ExitCodes::EXIT_SUCCESS); + $this->downloadDbCommand->method('run')->withAnyParameters()->willReturn(ExitCode::EXIT_SUCCESS); $this->commandTester->execute([], ['verbosity' => OutputInterface::VERBOSITY_VERBOSE]); $output = $this->commandTester->getDisplay(); @@ -186,7 +186,7 @@ class LocateVisitsCommandTest extends TestCase public function showsProperMessageWhenGeoLiteUpdateFails(): void { $this->lock->method('acquire')->with($this->isFalse())->willReturn(true); - $this->downloadDbCommand->method('run')->withAnyParameters()->willReturn(ExitCodes::EXIT_FAILURE); + $this->downloadDbCommand->method('run')->withAnyParameters()->willReturn(ExitCode::EXIT_FAILURE); $this->visitService->expects($this->never())->method('locateUnlocatedVisits'); $this->commandTester->execute([]); @@ -199,7 +199,7 @@ class LocateVisitsCommandTest extends TestCase public function providingAllFlagOnItsOwnDisplaysNotice(): void { $this->lock->method('acquire')->with($this->isFalse())->willReturn(true); - $this->downloadDbCommand->method('run')->withAnyParameters()->willReturn(ExitCodes::EXIT_SUCCESS); + $this->downloadDbCommand->method('run')->withAnyParameters()->willReturn(ExitCode::EXIT_SUCCESS); $this->commandTester->execute(['--all' => true]); $output = $this->commandTester->getDisplay(); @@ -210,7 +210,7 @@ class LocateVisitsCommandTest extends TestCase #[Test, DataProvider('provideAbortInputs')] public function processingAllCancelsCommandIfUserDoesNotActivelyAgreeToConfirmation(array $inputs): void { - $this->downloadDbCommand->method('run')->withAnyParameters()->willReturn(ExitCodes::EXIT_SUCCESS); + $this->downloadDbCommand->method('run')->withAnyParameters()->willReturn(ExitCode::EXIT_SUCCESS); $this->expectException(RuntimeException::class); $this->expectExceptionMessage('Execution aborted'); diff --git a/module/Core/src/ShortUrl/Model/ShortUrlIdentifier.php b/module/Core/src/ShortUrl/Model/ShortUrlIdentifier.php index bb3b4af6..78becbed 100644 --- a/module/Core/src/ShortUrl/Model/ShortUrlIdentifier.php +++ b/module/Core/src/ShortUrl/Model/ShortUrlIdentifier.php @@ -8,6 +8,8 @@ use Psr\Http\Message\ServerRequestInterface; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; use Symfony\Component\Console\Input\InputInterface; +use function sprintf; + final class ShortUrlIdentifier { private function __construct(public readonly string $shortCode, public readonly ?string $domain = null) @@ -54,4 +56,13 @@ final class ShortUrlIdentifier { return new self($shortCode, $domain); } + + public function __toString(): string + { + if ($this->domain === null) { + return $this->shortCode; + } + + return sprintf('%s/%s', $this->domain, $this->shortCode); + } } diff --git a/module/Core/src/ShortUrl/ShortUrlVisitsDeleter.php b/module/Core/src/ShortUrl/ShortUrlVisitsDeleter.php index c202c5c2..8ad6713f 100644 --- a/module/Core/src/ShortUrl/ShortUrlVisitsDeleter.php +++ b/module/Core/src/ShortUrl/ShortUrlVisitsDeleter.php @@ -21,7 +21,7 @@ class ShortUrlVisitsDeleter implements ShortUrlVisitsDeleterInterface /** * @throws ShortUrlNotFoundException */ - public function deleteShortUrlVisits(ShortUrlIdentifier $identifier, ?ApiKey $apiKey): BulkDeleteResult + public function deleteShortUrlVisits(ShortUrlIdentifier $identifier, ?ApiKey $apiKey = null): BulkDeleteResult { $shortUrl = $this->resolver->resolveShortUrl($identifier, $apiKey); return new BulkDeleteResult($this->repository->deleteShortUrlVisits($shortUrl)); diff --git a/module/Core/src/ShortUrl/ShortUrlVisitsDeleterInterface.php b/module/Core/src/ShortUrl/ShortUrlVisitsDeleterInterface.php index b0ac0e6a..46e9fde5 100644 --- a/module/Core/src/ShortUrl/ShortUrlVisitsDeleterInterface.php +++ b/module/Core/src/ShortUrl/ShortUrlVisitsDeleterInterface.php @@ -14,5 +14,5 @@ interface ShortUrlVisitsDeleterInterface /** * @throws ShortUrlNotFoundException */ - public function deleteShortUrlVisits(ShortUrlIdentifier $identifier, ?ApiKey $apiKey): BulkDeleteResult; + public function deleteShortUrlVisits(ShortUrlIdentifier $identifier, ?ApiKey $apiKey = null): BulkDeleteResult; } diff --git a/module/Core/test-db/Visit/Repository/VisitDeleterRepositoryTest.php b/module/Core/test-db/Visit/Repository/VisitDeleterRepositoryTest.php index 1598fc94..53b1585f 100644 --- a/module/Core/test-db/Visit/Repository/VisitDeleterRepositoryTest.php +++ b/module/Core/test-db/Visit/Repository/VisitDeleterRepositoryTest.php @@ -52,7 +52,6 @@ class VisitDeleterRepositoryTest extends DatabaseTestCase $this->getEntityManager()->flush(); - self::assertEquals(0, $this->repo->deleteShortUrlVisits(ShortUrl::withLongUrl('https://invalid')->setId('99'))); self::assertEquals(2, $this->repo->deleteShortUrlVisits($shortUrl1)); self::assertEquals(0, $this->repo->deleteShortUrlVisits($shortUrl1)); self::assertEquals(4, $this->repo->deleteShortUrlVisits($shortUrl2)); diff --git a/module/Rest/test-api/Action/CreateShortUrlTest.php b/module/Rest/test-api/Action/CreateShortUrlTest.php index 54d1d45a..dfdd170e 100644 --- a/module/Rest/test-api/Action/CreateShortUrlTest.php +++ b/module/Rest/test-api/Action/CreateShortUrlTest.php @@ -320,27 +320,6 @@ class CreateShortUrlTest extends ApiTestCase yield 'example domain' => ['example.com']; } - #[Test, DataProvider('provideTwitterUrls')] - public function urlsWithBotProtectionCanBeShortenedWithUrlValidationEnabled(string $longUrl): void - { - // Requests to Twitter are randomly failing from GitHub actions. Let's skip this test there. - // This is a deprecated and low-used feature anyway. - if (env('CI', false)) { - $this->markTestSkipped(); - } - - [$statusCode] = $this->createShortUrl(['longUrl' => $longUrl, 'validateUrl' => true]); - self::assertEquals(self::STATUS_OK, $statusCode); - } - - public static function provideTwitterUrls(): iterable - { - yield ['https://twitter.com/shlinkio']; - yield ['https://mobile.twitter.com/shlinkio']; - yield ['https://twitter.com/shlinkio/status/1360637738421268481']; - yield ['https://mobile.twitter.com/shlinkio/status/1360637738421268481']; - } - #[Test] public function canCreateShortUrlsWithEmojis(): void { From c7043af853c9dacc633320b3c7d96a8f24b1275e Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 16 May 2023 09:26:29 +0200 Subject: [PATCH 35/55] Create DeleteShortUrlVisitsCommandTest --- .../DeleteShortUrlVisitsCommandTest.php | 85 +++++++++++++++++++ 1 file changed, 85 insertions(+) create mode 100644 module/CLI/test/Command/ShortUrl/DeleteShortUrlVisitsCommandTest.php diff --git a/module/CLI/test/Command/ShortUrl/DeleteShortUrlVisitsCommandTest.php b/module/CLI/test/Command/ShortUrl/DeleteShortUrlVisitsCommandTest.php new file mode 100644 index 00000000..88c3657a --- /dev/null +++ b/module/CLI/test/Command/ShortUrl/DeleteShortUrlVisitsCommandTest.php @@ -0,0 +1,85 @@ +deleter = $this->createMock(ShortUrlVisitsDeleterInterface::class); + $this->commandTester = $this->testerForCommand(new DeleteShortUrlVisitsCommand($this->deleter)); + } + + #[Test, DataProvider('provideCancellingInputs')] + public function executionIsAbortedIfManuallyCancelled(array $input): void + { + $this->deleter->expects($this->never())->method('deleteShortUrlVisits'); + $this->commandTester->setInputs($input); + + $exitCode = $this->commandTester->execute(['shortCode' => 'foo']); + $output = $this->commandTester->getDisplay(); + + self::assertEquals(ExitCode::EXIT_SUCCESS, $exitCode); + self::assertStringContainsString('Operation aborted', $output); + } + + public static function provideCancellingInputs(): iterable + { + yield 'default input' => [[]]; + yield 'no' => [['no']]; + yield 'n' => [['n']]; + } + + #[Test, DataProvider('provideErrorArgs')] + public function warningIsPrintedInCaseOfNotFoundShortUrl(array $args, string $expectedError): void + { + $this->deleter->expects($this->once())->method('deleteShortUrlVisits')->willThrowException( + new ShortUrlNotFoundException(), + ); + $this->commandTester->setInputs(['yes']); + + $exitCode = $this->commandTester->execute($args); + $output = $this->commandTester->getDisplay(); + + self::assertEquals(ExitCode::EXIT_WARNING, $exitCode); + self::assertStringContainsString($expectedError, $output); + } + + public static function provideErrorArgs(): iterable + { + yield 'domain' => [['shortCode' => 'foo'], 'Short URL not found for "foo"']; + yield 'no domain' => [['shortCode' => 'foo', '--domain' => 's.test'], 'Short URL not found for "s.test/foo"']; + } + + #[Test] + public function successMessageIsPrintedForValidShortUrls(): void + { + $this->deleter->expects($this->once())->method('deleteShortUrlVisits')->willReturn(new BulkDeleteResult(5)); + $this->commandTester->setInputs(['yes']); + + $exitCode = $this->commandTester->execute(['shortCode' => 'foo']); + $output = $this->commandTester->getDisplay(); + + self::assertEquals(ExitCode::EXIT_SUCCESS, $exitCode); + self::assertStringContainsString('Successfully deleted 5 visits', $output); + } +} From 765199727ec6712069f51c474a8bf924a0be9516 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 16 May 2023 09:29:22 +0200 Subject: [PATCH 36/55] Update changelog --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ab9409d6..8439c1c9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,12 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this ## [Unreleased] ### Added +* [#1148](https://github.com/shlinkio/shlink/issues/1148) Add support to delete short URL visits. + + This can be done via `DELETE /short-urls/{shortCode}/visits` REST endpoint or via `short-url:delete-visits` console command. + + The CLI command includes a warning and requires the user to confirm before proceeding. + * [#1656](https://github.com/shlinkio/shlink/issues/1656) Add support for openswoole 22 ### Changed From 39095a309818cb29569ae736102922dd6dfe6bda Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Wed, 17 May 2023 08:57:36 +0200 Subject: [PATCH 37/55] Fix coding styles --- config/autoload/dependencies.global.php | 3 ++- module/Rest/test-api/Action/CreateShortUrlTest.php | 1 - 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/config/autoload/dependencies.global.php b/config/autoload/dependencies.global.php index 657caffb..a0014ef6 100644 --- a/config/autoload/dependencies.global.php +++ b/config/autoload/dependencies.global.php @@ -4,6 +4,7 @@ declare(strict_types=1); use GuzzleHttp\Client; use Laminas\ServiceManager\AbstractFactory\ConfigAbstractFactory; +use Mezzio\Application; use Mezzio\Container; use Psr\Http\Client\ClientInterface; use Psr\Http\Message\ServerRequestFactoryInterface; @@ -20,7 +21,7 @@ return [ ], 'delegators' => [ - Mezzio\Application::class => [ + Application::class => [ Container\ApplicationConfigInjectionDelegator::class, ], ], diff --git a/module/Rest/test-api/Action/CreateShortUrlTest.php b/module/Rest/test-api/Action/CreateShortUrlTest.php index dfdd170e..5b22e79a 100644 --- a/module/Rest/test-api/Action/CreateShortUrlTest.php +++ b/module/Rest/test-api/Action/CreateShortUrlTest.php @@ -12,7 +12,6 @@ use Shlinkio\Shlink\TestUtils\ApiTest\ApiTestCase; use function Functional\map; use function range; -use function Shlinkio\Shlink\Config\env; use function sprintf; class CreateShortUrlTest extends ApiTestCase From a4d8ebdfc95b2ae4698db2925b2f0aa1906b8efe Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 18 May 2023 08:57:24 +0200 Subject: [PATCH 38/55] Create DB logic to delete orphan visits --- composer.json | 2 +- .../Repository/VisitDeleterRepository.php | 9 +++++++++ .../VisitDeleterRepositoryInterface.php | 2 ++ .../Repository/VisitDeleterRepositoryTest.php | 19 ++++++++++++++++++- .../src/Action/Visit/OrphanVisitsAction.php | 4 ++-- 5 files changed, 32 insertions(+), 4 deletions(-) diff --git a/composer.json b/composer.json index b47d0dfc..c9d9757d 100644 --- a/composer.json +++ b/composer.json @@ -65,7 +65,7 @@ "require-dev": { "cebe/php-openapi": "^1.7", "devster/ubench": "^2.1", - "infection/infection": "^0.26.19", + "infection/infection": "^0.27", "openswoole/ide-helper": "~22.0.0", "phpstan/phpstan": "^1.9", "phpstan/phpstan-doctrine": "^1.3", diff --git a/module/Core/src/Visit/Repository/VisitDeleterRepository.php b/module/Core/src/Visit/Repository/VisitDeleterRepository.php index 602ba576..19f79dc7 100644 --- a/module/Core/src/Visit/Repository/VisitDeleterRepository.php +++ b/module/Core/src/Visit/Repository/VisitDeleterRepository.php @@ -19,4 +19,13 @@ class VisitDeleterRepository extends EntitySpecificationRepository implements Vi return $qb->getQuery()->execute(); } + + public function deleteOrphanVisits(): int + { + $qb = $this->getEntityManager()->createQueryBuilder(); + $qb->delete(Visit::class, 'v') + ->where($qb->expr()->isNull('v.shortUrl')); + + return $qb->getQuery()->execute(); + } } diff --git a/module/Core/src/Visit/Repository/VisitDeleterRepositoryInterface.php b/module/Core/src/Visit/Repository/VisitDeleterRepositoryInterface.php index 61a8af9b..a1516225 100644 --- a/module/Core/src/Visit/Repository/VisitDeleterRepositoryInterface.php +++ b/module/Core/src/Visit/Repository/VisitDeleterRepositoryInterface.php @@ -9,4 +9,6 @@ use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; interface VisitDeleterRepositoryInterface { public function deleteShortUrlVisits(ShortUrl $shortUrl): int; + + public function deleteOrphanVisits(): int; } diff --git a/module/Core/test-db/Visit/Repository/VisitDeleterRepositoryTest.php b/module/Core/test-db/Visit/Repository/VisitDeleterRepositoryTest.php index 53b1585f..62aa89e7 100644 --- a/module/Core/test-db/Visit/Repository/VisitDeleterRepositoryTest.php +++ b/module/Core/test-db/Visit/Repository/VisitDeleterRepositoryTest.php @@ -25,7 +25,7 @@ class VisitDeleterRepositoryTest extends DatabaseTestCase } #[Test] - public function deletesExpectedVisits(): void + public function deletesExpectedShortUrlVisits(): void { $shortUrl1 = ShortUrl::withLongUrl('https://foo.com'); $this->getEntityManager()->persist($shortUrl1); @@ -59,4 +59,21 @@ class VisitDeleterRepositoryTest extends DatabaseTestCase self::assertEquals(1, $this->repo->deleteShortUrlVisits($shortUrl3)); self::assertEquals(0, $this->repo->deleteShortUrlVisits($shortUrl3)); } + + #[Test] + public function deletesExpectedOrphanVisits(): void + { + $visitor = Visitor::emptyInstance(); + $this->getEntityManager()->persist(Visit::forBasePath($visitor)); + $this->getEntityManager()->persist(Visit::forInvalidShortUrl($visitor)); + $this->getEntityManager()->persist(Visit::forRegularNotFound($visitor)); + $this->getEntityManager()->persist(Visit::forBasePath($visitor)); + $this->getEntityManager()->persist(Visit::forInvalidShortUrl($visitor)); + $this->getEntityManager()->persist(Visit::forRegularNotFound($visitor)); + + $this->getEntityManager()->flush(); + + self::assertEquals(6, $this->repo->deleteOrphanVisits()); + self::assertEquals(0, $this->repo->deleteOrphanVisits()); + } } diff --git a/module/Rest/src/Action/Visit/OrphanVisitsAction.php b/module/Rest/src/Action/Visit/OrphanVisitsAction.php index e53c2a6f..af5292a2 100644 --- a/module/Rest/src/Action/Visit/OrphanVisitsAction.php +++ b/module/Rest/src/Action/Visit/OrphanVisitsAction.php @@ -21,8 +21,8 @@ class OrphanVisitsAction extends AbstractRestAction protected const ROUTE_ALLOWED_METHODS = [self::METHOD_GET]; public function __construct( - private VisitsStatsHelperInterface $visitsHelper, - private DataTransformerInterface $orphanVisitTransformer, + private readonly VisitsStatsHelperInterface $visitsHelper, + private readonly DataTransformerInterface $orphanVisitTransformer, ) { } From abcf2f86be8e27bb4e7af6709c9c694adb824a5d Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 18 May 2023 09:01:57 +0200 Subject: [PATCH 39/55] Create service to delete orphan visits --- module/Core/config/dependencies.config.php | 2 + module/Core/src/Visit/VisitsDeleter.php | 22 ++++++++++ .../Core/src/Visit/VisitsDeleterInterface.php | 13 ++++++ module/Core/test/Visit/VisitsDeleterTest.php | 41 +++++++++++++++++++ 4 files changed, 78 insertions(+) create mode 100644 module/Core/src/Visit/VisitsDeleter.php create mode 100644 module/Core/src/Visit/VisitsDeleterInterface.php create mode 100644 module/Core/test/Visit/VisitsDeleterTest.php diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index 8e3e9a52..8b6acc3e 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -62,6 +62,7 @@ return [ Visit\VisitsTracker::class => ConfigAbstractFactory::class, Visit\RequestTracker::class => ConfigAbstractFactory::class, + Visit\VisitsDeleter::class => ConfigAbstractFactory::class, Visit\Geolocation\VisitLocator::class => ConfigAbstractFactory::class, Visit\Geolocation\VisitToLocationHelper::class => ConfigAbstractFactory::class, Visit\VisitsStatsHelper::class => ConfigAbstractFactory::class, @@ -122,6 +123,7 @@ return [ Options\TrackingOptions::class, ], Visit\RequestTracker::class => [Visit\VisitsTracker::class, Options\TrackingOptions::class], + Visit\VisitsDeleter::class => [Visit\Repository\VisitDeleterRepository::class], ShortUrl\ShortUrlService::class => [ 'em', ShortUrl\ShortUrlResolver::class, diff --git a/module/Core/src/Visit/VisitsDeleter.php b/module/Core/src/Visit/VisitsDeleter.php new file mode 100644 index 00000000..5a8adca5 --- /dev/null +++ b/module/Core/src/Visit/VisitsDeleter.php @@ -0,0 +1,22 @@ +repository->deleteOrphanVisits()); + } +} diff --git a/module/Core/src/Visit/VisitsDeleterInterface.php b/module/Core/src/Visit/VisitsDeleterInterface.php new file mode 100644 index 00000000..3a75a0d3 --- /dev/null +++ b/module/Core/src/Visit/VisitsDeleterInterface.php @@ -0,0 +1,13 @@ +repo = $this->createMock(VisitDeleterRepositoryInterface::class); + $this->visitsDeleter = new VisitsDeleter($this->repo); + } + + #[Test, DataProvider('provideVisitsCounts')] + public function returnsDeletedVisitsFromRepo(int $visitsCount): void + { + $this->repo->expects($this->once())->method('deleteOrphanVisits')->willReturn($visitsCount); + + $result = $this->visitsDeleter->deleteOrphanVisits(); + + self::assertEquals($visitsCount, $result->affectedItems); + } + + public static function provideVisitsCounts(): iterable + { + yield '45' => [45]; + yield '5000' => [5000]; + yield '0' => [0]; + } +} From bdfb220126d0758dc36d1977fcaab766879e2bd8 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 18 May 2023 09:04:28 +0200 Subject: [PATCH 40/55] Create REST action to delete orphan visits --- config/autoload/routes.config.php | 1 + module/Rest/config/dependencies.config.php | 2 + .../Action/Visit/DeleteOrphanVisitsAction.php | 33 ++++++++++++ .../Visit/DeleteOrphanVisitsActionTest.php | 53 +++++++++++++++++++ 4 files changed, 89 insertions(+) create mode 100644 module/Rest/src/Action/Visit/DeleteOrphanVisitsAction.php create mode 100644 module/Rest/test/Action/Visit/DeleteOrphanVisitsActionTest.php diff --git a/config/autoload/routes.config.php b/config/autoload/routes.config.php index 93464519..ea305d86 100644 --- a/config/autoload/routes.config.php +++ b/config/autoload/routes.config.php @@ -38,6 +38,7 @@ return (static function (): array { Action\Visit\DomainVisitsAction::getRouteDef(), Action\Visit\GlobalVisitsAction::getRouteDef(), Action\Visit\OrphanVisitsAction::getRouteDef(), + Action\Visit\DeleteOrphanVisitsAction::getRouteDef(), Action\Visit\NonOrphanVisitsAction::getRouteDef(), // Short URLs diff --git a/module/Rest/config/dependencies.config.php b/module/Rest/config/dependencies.config.php index 43625c41..acca571d 100644 --- a/module/Rest/config/dependencies.config.php +++ b/module/Rest/config/dependencies.config.php @@ -38,6 +38,7 @@ return [ Action\Visit\DomainVisitsAction::class => ConfigAbstractFactory::class, Action\Visit\GlobalVisitsAction::class => ConfigAbstractFactory::class, Action\Visit\OrphanVisitsAction::class => ConfigAbstractFactory::class, + Action\Visit\DeleteOrphanVisitsAction::class => ConfigAbstractFactory::class, Action\Visit\NonOrphanVisitsAction::class => ConfigAbstractFactory::class, Action\Tag\ListTagsAction::class => ConfigAbstractFactory::class, Action\Tag\TagsStatsAction::class => ConfigAbstractFactory::class, @@ -90,6 +91,7 @@ return [ Visit\VisitsStatsHelper::class, Visit\Transformer\OrphanVisitDataTransformer::class, ], + Action\Visit\DeleteOrphanVisitsAction::class => [Visit\VisitsDeleter::class], Action\Visit\NonOrphanVisitsAction::class => [Visit\VisitsStatsHelper::class], Action\ShortUrl\ListShortUrlsAction::class => [ ShortUrl\ShortUrlListService::class, diff --git a/module/Rest/src/Action/Visit/DeleteOrphanVisitsAction.php b/module/Rest/src/Action/Visit/DeleteOrphanVisitsAction.php new file mode 100644 index 00000000..d1d2bc84 --- /dev/null +++ b/module/Rest/src/Action/Visit/DeleteOrphanVisitsAction.php @@ -0,0 +1,33 @@ +visitsDeleter->deleteOrphanVisits($apiKey); + + return new JsonResponse($result->toArray('deletedVisits')); + } +} diff --git a/module/Rest/test/Action/Visit/DeleteOrphanVisitsActionTest.php b/module/Rest/test/Action/Visit/DeleteOrphanVisitsActionTest.php new file mode 100644 index 00000000..b7f6031e --- /dev/null +++ b/module/Rest/test/Action/Visit/DeleteOrphanVisitsActionTest.php @@ -0,0 +1,53 @@ +deleter = $this->createMock(VisitsDeleterInterface::class); + $this->action = new DeleteOrphanVisitsAction($this->deleter); + } + + #[Test, DataProvider('provideVisitsCounts')] + public function orphanVisitsAreDeleted(int $visitsCount): void + { + $apiKey = ApiKey::create(); + $request = ServerRequestFactory::fromGlobals()->withAttribute(ApiKey::class, $apiKey); + + $this->deleter->expects($this->once())->method('deleteOrphanVisits')->with($apiKey)->willReturn( + new BulkDeleteResult($visitsCount), + ); + + /** @var JsonResponse $resp */ + $resp = $this->action->handle($request); + $payload = $resp->getPayload(); + + self::assertEquals(['deletedVisits' => $visitsCount], $payload); + } + + public static function provideVisitsCounts(): iterable + { + yield '1' => [1]; + yield '0' => [0]; + yield '300' => [300]; + yield '1234' => [1234]; + } +} From a6f0c6633182e6c9519356192d509b48e8c7a4c8 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 18 May 2023 09:06:52 +0200 Subject: [PATCH 41/55] Document endpoint to delete orphan visits --- docs/swagger/paths/v2_visits_orphan.json | 50 ++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/docs/swagger/paths/v2_visits_orphan.json b/docs/swagger/paths/v2_visits_orphan.json index b10ac37f..fe799934 100644 --- a/docs/swagger/paths/v2_visits_orphan.json +++ b/docs/swagger/paths/v2_visits_orphan.json @@ -148,5 +148,55 @@ } } } + }, + + "delete": { + "operationId": "deleteOrphanVisits", + "tags": [ + "Visits" + ], + "summary": "Delete orphan visits", + "description": "Delete all visits to invalid short URLs, the base URL or any other 404.", + "parameters": [ + { + "$ref": "../parameters/version.json" + } + ], + "security": [ + { + "ApiKey": [] + } + ], + "responses": { + "200": { + "description": "Deleted visits", + "content": { + "application/json": { + "schema": { + "type": "object", + "properties": { + "deletedVisits": { + "description": "Amount of affected visits", + "type": "number" + } + } + }, + "example": { + "deletedVisits": 536 + } + } + } + }, + "default": { + "description": "Unexpected error.", + "content": { + "application/problem+json": { + "schema": { + "$ref": "../definitions/Error.json" + } + } + } + } + } } } From 3916c6812614b7c2f041b02f67d0519d4d47e3a7 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 18 May 2023 09:09:44 +0200 Subject: [PATCH 42/55] Add DeleteOrphanVisitsTest API test --- .../Action/DeleteOrphanVisitsTest.php | 42 +++++++++++++++++++ .../Action/DeleteShortUrlVisitsTest.php | 4 +- 2 files changed, 44 insertions(+), 2 deletions(-) create mode 100644 module/Rest/test-api/Action/DeleteOrphanVisitsTest.php diff --git a/module/Rest/test-api/Action/DeleteOrphanVisitsTest.php b/module/Rest/test-api/Action/DeleteOrphanVisitsTest.php new file mode 100644 index 00000000..b7cf59b9 --- /dev/null +++ b/module/Rest/test-api/Action/DeleteOrphanVisitsTest.php @@ -0,0 +1,42 @@ +getTotalVisits()); + self::assertEquals(3, $this->getOrphanVisits()); + + $resp = $this->callApiWithKey(self::METHOD_DELETE, '/visits/orphan'); + $payload = $this->getJsonResponsePayload($resp); + + self::assertEquals(200, $resp->getStatusCode()); + self::assertEquals(3, $payload['deletedVisits']); + self::assertEquals(7, $this->getTotalVisits()); // This verifies that regular visits have not been affected + self::assertEquals(0, $this->getOrphanVisits()); + } + + private function getTotalVisits(): int + { + $resp = $this->callApiWithKey(self::METHOD_GET, '/visits/non-orphan'); + $payload = $this->getJsonResponsePayload($resp); + + return $payload['visits']['pagination']['totalItems']; + } + + private function getOrphanVisits(): int + { + $resp = $this->callApiWithKey(self::METHOD_GET, '/visits/orphan'); + $payload = $this->getJsonResponsePayload($resp); + + return $payload['visits']['pagination']['totalItems']; + } +} diff --git a/module/Rest/test-api/Action/DeleteShortUrlVisitsTest.php b/module/Rest/test-api/Action/DeleteShortUrlVisitsTest.php index 045f2c9a..7b5f306d 100644 --- a/module/Rest/test-api/Action/DeleteShortUrlVisitsTest.php +++ b/module/Rest/test-api/Action/DeleteShortUrlVisitsTest.php @@ -22,8 +22,8 @@ class DeleteShortUrlVisitsTest extends ApiTestCase self::assertEquals(200, $resp->getStatusCode()); self::assertEquals(3, $payload['deletedVisits']); - self::assertEquals(4, $this->getTotalVisits()); - self::assertEquals(3, $this->getOrphanVisits()); + self::assertEquals(4, $this->getTotalVisits()); // This verifies that other visits have not been affected + self::assertEquals(3, $this->getOrphanVisits()); // This verifies that orphan visits have not been affected } private function getTotalVisits(): int From 7f02243c6c5103a389dab1376a923be9b3eb2cdb Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 18 May 2023 09:19:01 +0200 Subject: [PATCH 43/55] Rename short-url:delete-visits to short-url:visits-delete for consistency with other commands --- CHANGELOG.md | 2 +- module/CLI/src/Command/ShortUrl/DeleteShortUrlVisitsCommand.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8439c1c9..81ca850c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this ### Added * [#1148](https://github.com/shlinkio/shlink/issues/1148) Add support to delete short URL visits. - This can be done via `DELETE /short-urls/{shortCode}/visits` REST endpoint or via `short-url:delete-visits` console command. + This can be done via `DELETE /short-urls/{shortCode}/visits` REST endpoint or via `short-url:visits-delete` console command. The CLI command includes a warning and requires the user to confirm before proceeding. diff --git a/module/CLI/src/Command/ShortUrl/DeleteShortUrlVisitsCommand.php b/module/CLI/src/Command/ShortUrl/DeleteShortUrlVisitsCommand.php index fa7a8ee6..5bb4d2a5 100644 --- a/module/CLI/src/Command/ShortUrl/DeleteShortUrlVisitsCommand.php +++ b/module/CLI/src/Command/ShortUrl/DeleteShortUrlVisitsCommand.php @@ -19,7 +19,7 @@ use function sprintf; class DeleteShortUrlVisitsCommand extends Command { - public const NAME = 'short-url:delete-visits'; + public const NAME = 'short-url:visits-delete'; public function __construct(private readonly ShortUrlVisitsDeleterInterface $deleter) { From 9d64d4ed1df462ab01b3a99fa6451560d64c42a3 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 18 May 2023 09:32:54 +0200 Subject: [PATCH 44/55] Create abstract base class for commands deleting visits --- .../ShortUrl/DeleteShortUrlVisitsCommand.php | 18 +++------- .../Visit/AbstractDeleteVisitsCommand.php | 35 +++++++++++++++++++ 2 files changed, 40 insertions(+), 13 deletions(-) create mode 100644 module/CLI/src/Command/Visit/AbstractDeleteVisitsCommand.php diff --git a/module/CLI/src/Command/ShortUrl/DeleteShortUrlVisitsCommand.php b/module/CLI/src/Command/ShortUrl/DeleteShortUrlVisitsCommand.php index 5bb4d2a5..6cd04bfe 100644 --- a/module/CLI/src/Command/ShortUrl/DeleteShortUrlVisitsCommand.php +++ b/module/CLI/src/Command/ShortUrl/DeleteShortUrlVisitsCommand.php @@ -4,20 +4,19 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI\Command\ShortUrl; +use Shlinkio\Shlink\CLI\Command\Visit\AbstractDeleteVisitsCommand; use Shlinkio\Shlink\CLI\Util\ExitCode; use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\ShortUrl\ShortUrlVisitsDeleterInterface; -use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputArgument; 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; -class DeleteShortUrlVisitsCommand extends Command +class DeleteShortUrlVisitsCommand extends AbstractDeleteVisitsCommand { public const NAME = 'short-url:visits-delete'; @@ -44,15 +43,9 @@ class DeleteShortUrlVisitsCommand extends Command ); } - protected function execute(InputInterface $input, OutputInterface $output): ?int + protected function doExecute(InputInterface $input, SymfonyStyle $io): ?int { $identifier = ShortUrlIdentifier::fromCli($input); - $io = new SymfonyStyle($input, $output); - if (! $this->confirm($io)) { - $io->info('Operation aborted'); - return ExitCode::EXIT_SUCCESS; - } - try { $result = $this->deleter->deleteShortUrlVisits($identifier); $io->success(sprintf('Successfully deleted %s visits', $result->affectedItems)); @@ -64,9 +57,8 @@ class DeleteShortUrlVisitsCommand extends Command } } - private function confirm(SymfonyStyle $io): bool + protected function getWarningMessage(): string { - $io->warning('You are about to delete all visits for a short URL. This operation cannot be undone.'); - return $io->confirm('Continue deleting visits?', false); + return 'You are about to delete all visits for a short URL. This operation cannot be undone.'; } } diff --git a/module/CLI/src/Command/Visit/AbstractDeleteVisitsCommand.php b/module/CLI/src/Command/Visit/AbstractDeleteVisitsCommand.php new file mode 100644 index 00000000..f171d59a --- /dev/null +++ b/module/CLI/src/Command/Visit/AbstractDeleteVisitsCommand.php @@ -0,0 +1,35 @@ +confirm($io)) { + $io->info('Operation aborted'); + return ExitCode::EXIT_SUCCESS; + } + + return $this->doExecute($input, $io); + } + + private function confirm(SymfonyStyle $io): bool + { + $io->warning($this->getWarningMessage()); + return $io->confirm('Continue deleting visits?', false); + } + + abstract protected function doExecute(InputInterface $input, SymfonyStyle $io): ?int; + + abstract protected function getWarningMessage(): string; +} From 618784dc3b7aa739969420ba05b73b85e6677499 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 18 May 2023 09:35:42 +0200 Subject: [PATCH 45/55] Create command to delete all orphan visits --- module/CLI/config/cli.config.php | 1 + module/CLI/config/dependencies.config.php | 2 + .../Visit/DeleteOrphanVisitsCommand.php | 42 ++++++++++++++++++ .../Visit/DeleteOrphanVisitsCommandTest.php | 43 +++++++++++++++++++ 4 files changed, 88 insertions(+) create mode 100644 module/CLI/src/Command/Visit/DeleteOrphanVisitsCommand.php create mode 100644 module/CLI/test/Command/Visit/DeleteOrphanVisitsCommandTest.php diff --git a/module/CLI/config/cli.config.php b/module/CLI/config/cli.config.php index 012c6800..9feeee7b 100644 --- a/module/CLI/config/cli.config.php +++ b/module/CLI/config/cli.config.php @@ -18,6 +18,7 @@ return [ Command\Visit\LocateVisitsCommand::NAME => Command\Visit\LocateVisitsCommand::class, Command\Visit\DownloadGeoLiteDbCommand::NAME => Command\Visit\DownloadGeoLiteDbCommand::class, Command\Visit\GetOrphanVisitsCommand::NAME => Command\Visit\GetOrphanVisitsCommand::class, + Command\Visit\DeleteOrphanVisitsCommand::NAME => Command\Visit\DeleteOrphanVisitsCommand::class, Command\Visit\GetNonOrphanVisitsCommand::NAME => Command\Visit\GetNonOrphanVisitsCommand::class, Command\Api\GenerateKeyCommand::NAME => Command\Api\GenerateKeyCommand::class, diff --git a/module/CLI/config/dependencies.config.php b/module/CLI/config/dependencies.config.php index 384df91d..6b7fc552 100644 --- a/module/CLI/config/dependencies.config.php +++ b/module/CLI/config/dependencies.config.php @@ -47,6 +47,7 @@ return [ Command\Visit\DownloadGeoLiteDbCommand::class => ConfigAbstractFactory::class, Command\Visit\LocateVisitsCommand::class => ConfigAbstractFactory::class, Command\Visit\GetOrphanVisitsCommand::class => ConfigAbstractFactory::class, + Command\Visit\DeleteOrphanVisitsCommand::class => ConfigAbstractFactory::class, Command\Visit\GetNonOrphanVisitsCommand::class => ConfigAbstractFactory::class, Command\Api\GenerateKeyCommand::class => ConfigAbstractFactory::class, @@ -98,6 +99,7 @@ return [ LockFactory::class, ], Command\Visit\GetOrphanVisitsCommand::class => [Visit\VisitsStatsHelper::class], + Command\Visit\DeleteOrphanVisitsCommand::class => [Visit\VisitsDeleter::class], Command\Visit\GetNonOrphanVisitsCommand::class => [Visit\VisitsStatsHelper::class, ShortUrlStringifier::class], Command\Api\GenerateKeyCommand::class => [ApiKeyService::class, ApiKey\RoleResolver::class], diff --git a/module/CLI/src/Command/Visit/DeleteOrphanVisitsCommand.php b/module/CLI/src/Command/Visit/DeleteOrphanVisitsCommand.php new file mode 100644 index 00000000..af1b7c66 --- /dev/null +++ b/module/CLI/src/Command/Visit/DeleteOrphanVisitsCommand.php @@ -0,0 +1,42 @@ +setName(self::NAME) + ->setDescription('Deletes all orphan visits'); + } + + protected function doExecute(InputInterface $input, SymfonyStyle $io): ?int + { + $result = $this->deleter->deleteOrphanVisits(); + $io->success(sprintf('Successfully deleted %s visits', $result->affectedItems)); + + return ExitCode::EXIT_SUCCESS; + } + + protected function getWarningMessage(): string + { + return 'You are about to delete all orphan visits. This operation cannot be undone.'; + } +} diff --git a/module/CLI/test/Command/Visit/DeleteOrphanVisitsCommandTest.php b/module/CLI/test/Command/Visit/DeleteOrphanVisitsCommandTest.php new file mode 100644 index 00000000..c18fe7f4 --- /dev/null +++ b/module/CLI/test/Command/Visit/DeleteOrphanVisitsCommandTest.php @@ -0,0 +1,43 @@ +deleter = $this->createMock(VisitsDeleterInterface::class); + $this->commandTester = $this->testerForCommand(new DeleteOrphanVisitsCommand($this->deleter)); + } + + #[Test] + public function successMessageIsPrintedAfterDeletion(): void + { + $this->deleter->expects($this->once())->method('deleteOrphanVisits')->willReturn(new BulkDeleteResult(5)); + $this->commandTester->setInputs(['yes']); + + $exitCode = $this->commandTester->execute([]); + $output = $this->commandTester->getDisplay(); + + self::assertEquals(ExitCode::EXIT_SUCCESS, $exitCode); + self::assertStringContainsString('You are about to delete all orphan visits.', $output); + self::assertStringContainsString('Successfully deleted 5 visits', $output); + } +} From d688c6da7e099152d8742feb725c9eaf85db8bd4 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 18 May 2023 09:36:50 +0200 Subject: [PATCH 46/55] Update changelog --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 81ca850c..d0a75254 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,12 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this The CLI command includes a warning and requires the user to confirm before proceeding. +* [#1681](https://github.com/shlinkio/shlink/issues/1681) Add support to delete orphan visits. + + This can be done via `DELETE /visits/orphan` REST endpoint or via `visit:orphan-delete` console command. + + The CLI command includes a warning and requires the user to confirm before proceeding. + * [#1656](https://github.com/shlinkio/shlink/issues/1656) Add support for openswoole 22 ### Changed From f03b7689cec056a29f77efb80780df6f0e6ed312 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 19 May 2023 19:38:36 +0200 Subject: [PATCH 47/55] Allow running docker container as non-root --- .github/workflows/publish-docker-image.yml | 5 +++++ Dockerfile | 19 +++++++------------ docker/docker-entrypoint.sh | 4 ++-- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/.github/workflows/publish-docker-image.yml b/.github/workflows/publish-docker-image.yml index 6beb9ac1..3dda2ead 100644 --- a/.github/workflows/publish-docker-image.yml +++ b/.github/workflows/publish-docker-image.yml @@ -28,6 +28,10 @@ jobs: - runtime: 'openswoole' tag-suffix: 'openswoole' platforms: 'linux/arm/v7,linux/arm64/v8,linux/amd64' + - runtime: 'rr' + tag-suffix: 'non-root' + platforms: 'linux/arm64/v8,linux/amd64' + user-id: '1001' uses: shlinkio/github-actions/.github/workflows/docker-build-and-publish.yml@main secrets: inherit with: @@ -37,3 +41,4 @@ jobs: tags-suffix: ${{ matrix.tag-suffix }} extra-build-args: | SHLINK_RUNTIME=${{ matrix.runtime }} + SHLINK_USER_ID=${{ matrix.user-id && matrix.user-id || 'root' }} diff --git a/Dockerfile b/Dockerfile index 10897176..4637e09e 100644 --- a/Dockerfile +++ b/Dockerfile @@ -4,11 +4,14 @@ ARG SHLINK_VERSION=latest ENV SHLINK_VERSION ${SHLINK_VERSION} ARG SHLINK_RUNTIME=rr ENV SHLINK_RUNTIME ${SHLINK_RUNTIME} +ARG SHLINK_USER_ID='root' +ENV SHLINK_USER_ID ${SHLINK_USER_ID} + ENV OPENSWOOLE_VERSION 22.0.0 ENV PDO_SQLSRV_VERSION 5.10.1 ENV MS_ODBC_DOWNLOAD 'b/9/f/b9f3cce4-3925-46d4-9f46-da08869c6486' ENV MS_ODBC_SQL_VERSION 18_18.1.1.1 -ENV LC_ALL "C" +ENV LC_ALL 'C' WORKDIR /etc/shlink @@ -48,7 +51,7 @@ RUN apk add --no-cache git && \ if [ "$SHLINK_RUNTIME" == 'openswoole' ]; then \ php composer.phar remove spiral/roadrunner spiral/roadrunner-jobs spiral/roadrunner-cli spiral/roadrunner-http --with-all-dependencies --update-no-dev --optimize-autoloader --no-progress --no-interaction ; \ elif [ "$SHLINK_RUNTIME" == 'rr' ]; then \ - php composer.phar remove mezzio/mezzio-swoole --with-all-dependencies --update-no-dev --optimize-autoloader --no-progress --no-interaction ; \ + php composer.phar remove mezzio/mezzio-swoole --with-all-dependencies --update-no-dev --optimize-autoloader --no-progress --no-interaction --ignore-platform-req=ext-openswoole ; \ fi; \ php composer.phar clear-cache && \ rm -r docker composer.* && \ @@ -59,7 +62,7 @@ RUN apk add --no-cache git && \ FROM base LABEL maintainer="Alejandro Celaya " -COPY --from=builder /etc/shlink . +COPY --from=builder --chown=${SHLINK_USER_ID} /etc/shlink . RUN ln -s /etc/shlink/bin/cli /usr/local/bin/shlink && \ if [ "$SHLINK_RUNTIME" == 'rr' ]; then \ php ./vendor/bin/rr get --no-interaction --no-config --location bin/ && chmod +x bin/rr ; \ @@ -73,14 +76,6 @@ COPY docker/docker-entrypoint.sh docker-entrypoint.sh COPY docker/config/shlink_in_docker.local.php config/autoload/shlink_in_docker.local.php COPY docker/config/php.ini ${PHP_INI_DIR}/conf.d/ -# Change the ownership of /etc/shlink/data to be writable, then change the user to non-root -# FIXME Disabled for now, as it conflicts with ENABLE_PERIODIC_VISIT_LOCATE, which is used to configure a cron as root. -# Ref: https://github.com/shlinkio/shlink/issues/1132 -#RUN chown 1001 /etc/shlink/data -#RUN chown 1001 /etc/shlink/data/locks -#RUN chown 1001 /etc/shlink/data/proxies -#RUN chown 1001 /etc/shlink/data/cache -#RUN chown 1001 /etc/shlink/data/log -#USER 1001 +USER ${SHLINK_USER_ID} ENTRYPOINT ["/bin/sh", "./docker-entrypoint.sh"] diff --git a/docker/docker-entrypoint.sh b/docker/docker-entrypoint.sh index fb8b7bf2..642bda7d 100644 --- a/docker/docker-entrypoint.sh +++ b/docker/docker-entrypoint.sh @@ -24,8 +24,8 @@ if [ ! -z "${GEOLITE_LICENSE_KEY}" ] && [ "${SKIP_INITIAL_GEOLITE_DOWNLOAD}" != php bin/cli visit:download-db -n ${flags} fi -# Periodically run visit:locate every hour, if ENABLE_PERIODIC_VISIT_LOCATE=true was provided -if [ "${ENABLE_PERIODIC_VISIT_LOCATE}" = "true" ]; then +# Periodically run visit:locate every hour, if ENABLE_PERIODIC_VISIT_LOCATE=true was provided and running as root +if [ "${ENABLE_PERIODIC_VISIT_LOCATE}" = "true" ] && [ "${SHLINK_USER_ID}" = "root" ]; then echo "Configuring periodic visit location..." echo "0 * * * * php /etc/shlink/bin/cli visit:locate -q" > /etc/crontabs/root /usr/sbin/crond & From 725370704f7aa3016054b688101f4bbe5eeaa8be Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 19 May 2023 19:50:05 +0200 Subject: [PATCH 48/55] Update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d0a75254..f3140a92 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this The CLI command includes a warning and requires the user to confirm before proceeding. * [#1656](https://github.com/shlinkio/shlink/issues/1656) Add support for openswoole 22 +* [#1784](https://github.com/shlinkio/shlink/issues/1784) Add new docker tag where the container runs as a non-root user. ### Changed * [#1755](https://github.com/shlinkio/shlink/issues/1755) Update to roadrunner 2023 From 794d926e3a260e20bbe5988cb54269fc73c714cc Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 21 May 2023 14:30:20 +0200 Subject: [PATCH 49/55] Update docker entry point to use new shlink-installer init command --- composer.json | 2 +- docker/docker-entrypoint.sh | 24 ++++++------------------ 2 files changed, 7 insertions(+), 19 deletions(-) diff --git a/composer.json b/composer.json index c9d9757d..1aa700cf 100644 --- a/composer.json +++ b/composer.json @@ -49,7 +49,7 @@ "shlinkio/shlink-config": "^2.4", "shlinkio/shlink-event-dispatcher": "dev-main#8c677ae as 3.0", "shlinkio/shlink-importer": "dev-main#6b63b12 as 5.1", - "shlinkio/shlink-installer": "dev-develop#0e015f8 as 8.4", + "shlinkio/shlink-installer": "dev-develop#9b7a090 as 8.4", "shlinkio/shlink-ip-geolocation": "^3.2", "shlinkio/shlink-json": "^1.0", "spiral/roadrunner": "^2023.1", diff --git a/docker/docker-entrypoint.sh b/docker/docker-entrypoint.sh index 642bda7d..0eb4b718 100644 --- a/docker/docker-entrypoint.sh +++ b/docker/docker-entrypoint.sh @@ -1,29 +1,17 @@ #!/usr/bin/env sh set -e -# If SHELL_VERBOSITY was not explicitly provided, run commands in quite mode (-q) -[ $SHELL_VERBOSITY ] && flags="" || flags="-q" - cd /etc/shlink -echo "Creating fresh database if needed..." -php bin/cli db:create -n ${flags} +flags="--clear-db-cache" -echo "Updating database..." -php bin/cli db:migrate -n ${flags} - -echo "Generating proxies..." -php bin/doctrine orm:generate-proxies -n ${flags} - -echo "Clearing entities cache..." -php bin/doctrine orm:clear-cache:metadata -n ${flags} - -# Try to download GeoLite2 db file only if the license key env var was defined and skipping was not explicitly set -if [ ! -z "${GEOLITE_LICENSE_KEY}" ] && [ "${SKIP_INITIAL_GEOLITE_DOWNLOAD}" != "true" ]; then - echo "Downloading GeoLite2 db file..." - php bin/cli visit:download-db -n ${flags} +# Skip downloading GeoLite2 db file if the license key env var was not defined or skipping was explicitly set +if [ -z "${GEOLITE_LICENSE_KEY}" ] || [ "${SKIP_INITIAL_GEOLITE_DOWNLOAD}" == "true" ]; then + flags="${flags} --skip-download-geolite" fi +php vendor/bin/shlink-installer init ${flags} + # Periodically run visit:locate every hour, if ENABLE_PERIODIC_VISIT_LOCATE=true was provided and running as root if [ "${ENABLE_PERIODIC_VISIT_LOCATE}" = "true" ] && [ "${SHLINK_USER_ID}" = "root" ]; then echo "Configuring periodic visit location..." From 90f93ee4ec726e2f6816679aa6a31654ec742b3a Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 21 May 2023 14:32:00 +0200 Subject: [PATCH 50/55] Update changelog --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f3140a92..9d0017fd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,12 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this The CLI command includes a warning and requires the user to confirm before proceeding. +* [#1753](https://github.com/shlinkio/shlink/issues/1753) Add a new `vendor/bin/shlink-installer init` command that can be used to automate Shlink installations. + + This command can create the initial database, update it, create proxies, clean cache, download initial GeoLite db files, etc + + The official docker image also uses it on its entry point script. + * [#1656](https://github.com/shlinkio/shlink/issues/1656) Add support for openswoole 22 * [#1784](https://github.com/shlinkio/shlink/issues/1784) Add new docker tag where the container runs as a non-root user. From e85d59c5a4957d19e175355cfa59db6d8103932f Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 21 May 2023 18:08:17 +0200 Subject: [PATCH 51/55] Add locks when creating short URL dependencies, to avoid race condition --- module/Core/config/dependencies.config.php | 7 ++- .../PersistenceShortUrlRelationResolver.php | 55 ++++++++++++++++++- ...ersistenceShortUrlRelationResolverTest.php | 8 ++- 3 files changed, 64 insertions(+), 6 deletions(-) diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index 8b6acc3e..da653406 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -13,6 +13,7 @@ use Shlinkio\Shlink\Core\ErrorHandler; use Shlinkio\Shlink\Core\Options\NotFoundRedirectOptions; use Shlinkio\Shlink\Importer\ImportedLinksProcessorInterface; use Shlinkio\Shlink\IpGeolocation\Resolver\IpLocationResolverInterface; +use Symfony\Component\Lock; return [ @@ -172,7 +173,11 @@ return [ ], Action\RobotsAction::class => [Crawling\CrawlingHelper::class], - ShortUrl\Resolver\PersistenceShortUrlRelationResolver::class => ['em', Options\UrlShortenerOptions::class], + ShortUrl\Resolver\PersistenceShortUrlRelationResolver::class => [ + 'em', + Options\UrlShortenerOptions::class, + Lock\LockFactory::class, + ], ShortUrl\Helper\ShortUrlStringifier::class => ['config.url_shortener.domain', 'config.router.base_path'], ShortUrl\Helper\ShortUrlTitleResolutionHelper::class => [Util\UrlValidator::class], ShortUrl\Helper\ShortUrlRedirectionBuilder::class => [Options\TrackingOptions::class], diff --git a/module/Core/src/ShortUrl/Resolver/PersistenceShortUrlRelationResolver.php b/module/Core/src/ShortUrl/Resolver/PersistenceShortUrlRelationResolver.php index 48ec634e..17669f32 100644 --- a/module/Core/src/ShortUrl/Resolver/PersistenceShortUrlRelationResolver.php +++ b/module/Core/src/ShortUrl/Resolver/PersistenceShortUrlRelationResolver.php @@ -11,7 +11,11 @@ use Doctrine\ORM\Events; use Shlinkio\Shlink\Core\Domain\Entity\Domain; use Shlinkio\Shlink\Core\Options\UrlShortenerOptions; use Shlinkio\Shlink\Core\Tag\Entity\Tag; +use Symfony\Component\Lock\Lock; +use Symfony\Component\Lock\LockFactory; +use Symfony\Component\Lock\Store\InMemoryStore; +use function Functional\invoke; use function Functional\map; use function Functional\unique; @@ -21,10 +25,15 @@ class PersistenceShortUrlRelationResolver implements ShortUrlRelationResolverInt private array $memoizedNewDomains = []; /** @var array */ private array $memoizedNewTags = []; + /** @var array */ + private array $tagLocks = []; + /** @var array */ + private array $domainLocks = []; public function __construct( private readonly EntityManagerInterface $em, private readonly UrlShortenerOptions $options = new UrlShortenerOptions(), + private readonly LockFactory $locker = new LockFactory(new InMemoryStore()), ) { // Registering this as an event listener will make the postFlush method to be called automatically $this->em->getEventManager()->addEventListener(Events::postFlush, $this); @@ -36,11 +45,18 @@ class PersistenceShortUrlRelationResolver implements ShortUrlRelationResolverInt return null; } + $this->lock($this->domainLocks, 'domain_' . $domain); + /** @var Domain|null $existingDomain */ $existingDomain = $this->em->getRepository(Domain::class)->findOneBy(['authority' => $domain]); + if ($existingDomain) { + // The lock can be released immediately of the domain is not new + $this->releaseLock($this->domainLocks, 'domain_' . $domain); + return $existingDomain; + } // Memoize only new domains, and let doctrine handle objects hydrated from persistence - return $existingDomain ?? $this->memoizeNewDomain($domain); + return $this->memoizeNewDomain($domain); } private function memoizeNewDomain(string $domain): Domain @@ -62,8 +78,16 @@ class PersistenceShortUrlRelationResolver implements ShortUrlRelationResolverInt $repo = $this->em->getRepository(Tag::class); return new Collections\ArrayCollection(map($tags, function (string $tagName) use ($repo): Tag { + $this->lock($this->tagLocks, 'tag_' . $tagName); + + $existingTag = $repo->findOneBy(['name' => $tagName]); + if ($existingTag) { + $this->releaseLock($this->tagLocks, 'tag_' . $tagName); + return $existingTag; + } + // Memoize only new tags, and let doctrine handle objects hydrated from persistence - $tag = $repo->findOneBy(['name' => $tagName]) ?? $this->memoizeNewTag($tagName); + $tag = $this->memoizeNewTag($tagName); $this->em->persist($tag); return $tag; @@ -75,9 +99,36 @@ class PersistenceShortUrlRelationResolver implements ShortUrlRelationResolverInt return $this->memoizedNewTags[$tagName] ??= new Tag($tagName); } + /** + * @param array $locks + */ + private function lock(array &$locks, string $name): void + { + // Lock dependency creation for up to 5 seconds. This will prevent errors when trying to create the same one + // more than once in parallel. + $locks[$name] = $lock = $this->locker->createLock($name, 5); + $lock->acquire(true); + } + + /** + * @param array $locks + */ + private function releaseLock(array &$locks, string $name): void + { + $locks[$name]->release(); + unset($locks[$name]); + } + public function postFlush(): void { + // Reset memoized domains and tags $this->memoizedNewDomains = []; $this->memoizedNewTags = []; + + // Release all locks + invoke($this->tagLocks, 'release'); + invoke($this->domainLocks, 'release'); + $this->tagLocks = []; + $this->domainLocks = []; } } diff --git a/module/Core/test/ShortUrl/Resolver/PersistenceShortUrlRelationResolverTest.php b/module/Core/test/ShortUrl/Resolver/PersistenceShortUrlRelationResolverTest.php index d31f3d9b..d7af118d 100644 --- a/module/Core/test/ShortUrl/Resolver/PersistenceShortUrlRelationResolverTest.php +++ b/module/Core/test/ShortUrl/Resolver/PersistenceShortUrlRelationResolverTest.php @@ -74,10 +74,12 @@ class PersistenceShortUrlRelationResolverTest extends TestCase #[Test, DataProvider('provideTags')] public function findsAndPersistsTagsWrappedIntoCollection(array $tags, array $expectedTags): void { - $expectedPersistedTags = count($expectedTags); + $expectedLookedOutTags = count($expectedTags); + // One of the tags will already exist. The rest will be new + $expectedPersistedTags = $expectedLookedOutTags - 1; $tagRepo = $this->createMock(TagRepositoryInterface::class); - $tagRepo->expects($this->exactly($expectedPersistedTags))->method('findOneBy')->with( + $tagRepo->expects($this->exactly($expectedLookedOutTags))->method('findOneBy')->with( $this->isType('array'), )->willReturnCallback(function (array $criteria): ?Tag { ['name' => $name] = $criteria; @@ -90,7 +92,7 @@ class PersistenceShortUrlRelationResolverTest extends TestCase $result = $this->resolver->resolveTags($tags); - self::assertCount($expectedPersistedTags, $result); + self::assertCount($expectedLookedOutTags, $result); self::assertEquals($expectedTags, $result->toArray()); } From 9743c1624d473f60ebe979b283533c85c6f68742 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sun, 21 May 2023 18:10:08 +0200 Subject: [PATCH 52/55] Update changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9d0017fd..4d41319e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this * [#1656](https://github.com/shlinkio/shlink/issues/1656) Add support for openswoole 22 * [#1784](https://github.com/shlinkio/shlink/issues/1784) Add new docker tag where the container runs as a non-root user. +* [#953](https://github.com/shlinkio/shlink/issues/953) Add locks that prevent errors on duplicated keys when creating short URLs in parallel that depend on the same new tag or domain. ### Changed * [#1755](https://github.com/shlinkio/shlink/issues/1755) Update to roadrunner 2023 @@ -45,6 +46,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this ### Fixed * [#1760](https://github.com/shlinkio/shlink/issues/1760) Fix domain not being set to null when importing short URLs with default domain. +* [#953](https://github.com/shlinkio/shlink/issues/953) Fix duplicated key errors and short URL creation failing when creating short URLs in parallel that depend on the same new tag or domain. * Fix Shlink trying to connect to RabbitMQ even if configuration set to not connect. From 882d64ae11d693fa23e7afebdf6713d36d7c75e2 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 23 May 2023 10:55:49 +0200 Subject: [PATCH 53/55] Add deprecation note for ENABLE_PERIODIC_VISIT_LOCATE env var --- docker/docker-entrypoint.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/docker/docker-entrypoint.sh b/docker/docker-entrypoint.sh index 0eb4b718..a2daec3d 100644 --- a/docker/docker-entrypoint.sh +++ b/docker/docker-entrypoint.sh @@ -13,6 +13,7 @@ fi php vendor/bin/shlink-installer init ${flags} # Periodically run visit:locate every hour, if ENABLE_PERIODIC_VISIT_LOCATE=true was provided and running as root +# ENABLE_PERIODIC_VISIT_LOCATE is deprecated. Remove cron support in Shlink 4.0.0 if [ "${ENABLE_PERIODIC_VISIT_LOCATE}" = "true" ] && [ "${SHLINK_USER_ID}" = "root" ]; then echo "Configuring periodic visit location..." echo "0 * * * * php /etc/shlink/bin/cli visit:locate -q" > /etc/crontabs/root From 096d2098d61b280900640d63a020b15d38e3961c Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 23 May 2023 18:42:50 +0200 Subject: [PATCH 54/55] Update installer --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 1aa700cf..f5407f99 100644 --- a/composer.json +++ b/composer.json @@ -49,7 +49,7 @@ "shlinkio/shlink-config": "^2.4", "shlinkio/shlink-event-dispatcher": "dev-main#8c677ae as 3.0", "shlinkio/shlink-importer": "dev-main#6b63b12 as 5.1", - "shlinkio/shlink-installer": "dev-develop#9b7a090 as 8.4", + "shlinkio/shlink-installer": "dev-develop#3420434 as 8.4", "shlinkio/shlink-ip-geolocation": "^3.2", "shlinkio/shlink-json": "^1.0", "spiral/roadrunner": "^2023.1", From 8c1865c3ec9706731d39c872259f6996fc047e13 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 23 May 2023 23:15:35 +0200 Subject: [PATCH 55/55] Update changelog --- CHANGELOG.md | 3 ++- composer.json | 8 ++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4d41319e..ebc43101 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ 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). -## [Unreleased] +## [3.6.0] - 2023-05-24 ### Added * [#1148](https://github.com/shlinkio/shlink/issues/1148) Add support to delete short URL visits. @@ -47,6 +47,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this ### Fixed * [#1760](https://github.com/shlinkio/shlink/issues/1760) Fix domain not being set to null when importing short URLs with default domain. * [#953](https://github.com/shlinkio/shlink/issues/953) Fix duplicated key errors and short URL creation failing when creating short URLs in parallel that depend on the same new tag or domain. +* [#1741](https://github.com/shlinkio/shlink/issues/1741) Fix randomly using 100% CPU in task workers when trying to download GeoLite DB files. * Fix Shlink trying to connect to RabbitMQ even if configuration set to not connect. diff --git a/composer.json b/composer.json index f5407f99..a3494086 100644 --- a/composer.json +++ b/composer.json @@ -45,11 +45,11 @@ "php-middleware/request-id": "^4.1", "pugx/shortid-php": "^1.1", "ramsey/uuid": "^4.7", - "shlinkio/shlink-common": "dev-main#88a34f1 as 5.5", + "shlinkio/shlink-common": "^5.5", "shlinkio/shlink-config": "^2.4", - "shlinkio/shlink-event-dispatcher": "dev-main#8c677ae as 3.0", - "shlinkio/shlink-importer": "dev-main#6b63b12 as 5.1", - "shlinkio/shlink-installer": "dev-develop#3420434 as 8.4", + "shlinkio/shlink-event-dispatcher": "^3.0", + "shlinkio/shlink-importer": "^5.1", + "shlinkio/shlink-installer": "^8.4", "shlinkio/shlink-ip-geolocation": "^3.2", "shlinkio/shlink-json": "^1.0", "spiral/roadrunner": "^2023.1",