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 b4335cb2..bdc5a025 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 }} @@ -69,8 +69,8 @@ jobs: with: php-version: ${{ matrix.php-version }} tools: composer - - run: composer install --no-interaction --prefer-dist - - run: ./vendor/bin/rr get --no-interaction --location bin/ && chmod +x bin/rr + - 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: composer test:api:rr sqlite-db-tests: @@ -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-docker-image.yml b/.github/workflows/publish-docker-image.yml index 7fb52fe1..3dda2ead 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,35 @@ 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' + 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: 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 }} + SHLINK_USER_ID=${{ matrix.user-id && matrix.user-id || 'root' }} diff --git a/.github/workflows/publish-release.yml b/.github/workflows/publish-release.yml index 8d8a4b0d..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' }} @@ -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-* diff --git a/.github/workflows/publish-swagger-spec.yml b/.github/workflows/publish-swagger-spec.yml index df2fd9c9..f7be6502 100644 --- a/.github/workflows/publish-swagger-spec.yml +++ b/.github/workflows/publish-swagger-spec.yml @@ -20,13 +20,13 @@ 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 }} - 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' 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/CHANGELOG.md b/CHANGELOG.md index 51f1facc..ebc43101 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,53 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com), and this project adheres to [Semantic Versioning](https://semver.org). +## [3.6.0] - 2023-05-24 +### 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:visits-delete` console command. + + 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. + +* [#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. +* [#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 +* [#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* + +### Removed +* *Nothing* + +### 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. + + ## [3.5.4] - 2023-04-12 ### Added * *Nothing* diff --git a/Dockerfile b/Dockerfile index 935c3d44..4637e09e 100644 --- a/Dockerfile +++ b/Dockerfile @@ -2,13 +2,16 @@ 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 4.12.1 +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 @@ -43,11 +46,12 @@ 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 --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 ; \ + 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 --ignore-platform-req=ext-openswoole ; \ fi; \ php composer.phar clear-cache && \ rm -r docker composer.* && \ @@ -58,10 +62,10 @@ 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 --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 @@ -72,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/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 03ff1457..a3494086 100644 --- a/composer.json +++ b/composer.json @@ -45,14 +45,17 @@ "php-middleware/request-id": "^4.1", "pugx/shortid-php": "^1.1", "ramsey/uuid": "^4.7", - "shlinkio/shlink-common": "^5.4", + "shlinkio/shlink-common": "^5.5", "shlinkio/shlink-config": "^2.4", - "shlinkio/shlink-event-dispatcher": "^2.6", - "shlinkio/shlink-importer": "^5.0", - "shlinkio/shlink-installer": "^8.3", + "shlinkio/shlink-event-dispatcher": "^3.0", + "shlinkio/shlink-importer": "^5.1", + "shlinkio/shlink-installer": "^8.4", "shlinkio/shlink-ip-geolocation": "^3.2", - "spiral/roadrunner": "^2.12", - "spiral/roadrunner-jobs": "^2.7", + "shlinkio/shlink-json": "^1.0", + "spiral/roadrunner": "^2023.1", + "spiral/roadrunner-cli": "^2.5", + "spiral/roadrunner-http": "^3.0", + "spiral/roadrunner-jobs": "^4.0", "symfony/console": "^6.2", "symfony/filesystem": "^6.2", "symfony/lock": "^6.2", @@ -62,17 +65,17 @@ "require-dev": { "cebe/php-openapi": "^1.7", "devster/ubench": "^2.1", - "infection/infection": "^0.26.19", - "openswoole/ide-helper": "~4.11.5", + "infection/infection": "^0.27", + "openswoole/ide-helper": "~22.0.0", "phpstan/phpstan": "^1.9", "phpstan/phpstan-doctrine": "^1.3", "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", + "shlinkio/shlink-test-utils": "^3.6", "symfony/var-dumper": "^6.2", "veewee/composer-run-parallel": "^1.2" }, @@ -107,7 +110,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/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/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/autoload/routes.config.php b/config/autoload/routes.config.php index c887d5b7..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 @@ -53,6 +54,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/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/config/roadrunner/.rr.dev.yml b/config/roadrunner/.rr.dev.yml index cc0bbf29..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: @@ -31,19 +33,8 @@ logs: mode: development channels: http: - level: debug + mode: 'off' # Disable logging as Shlink handles it internally server: 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 d44801ee..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 @@ -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/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 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 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', ], ], diff --git a/docker/docker-entrypoint.sh b/docker/docker-entrypoint.sh index fb8b7bf2..a2daec3d 100644 --- a/docker/docker-entrypoint.sh +++ b/docker/docker-entrypoint.sh @@ -1,31 +1,20 @@ #!/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 -# Periodically run visit:locate every hour, if ENABLE_PERIODIC_VISIT_LOCATE=true was provided -if [ "${ENABLE_PERIODIC_VISIT_LOCATE}" = "true" ]; then +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 /usr/sbin/crond & 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/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" + } + } + } + } + } } } 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": { diff --git a/module/CLI/config/cli.config.php b/module/CLI/config/cli.config.php index 7629d855..9feeee7b 100644 --- a/module/CLI/config/cli.config.php +++ b/module/CLI/config/cli.config.php @@ -13,10 +13,12 @@ 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, 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 e5176f42..6b7fc552 100644 --- a/module/CLI/config/dependencies.config.php +++ b/module/CLI/config/dependencies.config.php @@ -42,10 +42,12 @@ 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, Command\Visit\GetOrphanVisitsCommand::class => ConfigAbstractFactory::class, + Command\Visit\DeleteOrphanVisitsCommand::class => ConfigAbstractFactory::class, Command\Visit\GetNonOrphanVisitsCommand::class => ConfigAbstractFactory::class, Command\Api\GenerateKeyCommand::class => ConfigAbstractFactory::class, @@ -88,6 +90,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 => [ @@ -96,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/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 bb332d82..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; @@ -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,19 +135,13 @@ 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); $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(...))(','); @@ -185,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..6cd04bfe --- /dev/null +++ b/module/CLI/src/Command/ShortUrl/DeleteShortUrlVisitsCommand.php @@ -0,0 +1,64 @@ +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 doExecute(InputInterface $input, SymfonyStyle $io): ?int + { + $identifier = ShortUrlIdentifier::fromCli($input); + 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; + } + } + + protected function getWarningMessage(): string + { + return 'You are about to delete all visits for a short URL. This operation cannot be undone.'; + } +} diff --git a/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php b/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php index 7a9c77af..14ea1851 100644 --- a/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php +++ b/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php @@ -4,9 +4,9 @@ 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\Util\ExitCodes; +use Shlinkio\Shlink\CLI\Input\EndDateOption; +use Shlinkio\Shlink\CLI\Input\StartDateOption; +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; @@ -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', @@ -167,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( @@ -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/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/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; +} diff --git a/module/CLI/src/Command/Visit/AbstractVisitsListCommand.php b/module/CLI/src/Command/Visit/AbstractVisitsListCommand.php index 402d5ba4..ba518656 100644 --- a/module/CLI/src/Command/Visit/AbstractVisitsListCommand.php +++ b/module/CLI/src/Command/Visit/AbstractVisitsListCommand.php @@ -4,9 +4,9 @@ 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\Util\ExitCodes; +use Shlinkio\Shlink\CLI\Input\EndDateOption; +use Shlinkio\Shlink\CLI\Input\StartDateOption; +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/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/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/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; 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 new file mode 100644 index 00000000..c2e96611 --- /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(ExitCode::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); + } +} 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/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); + } +} 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 fd474007..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; @@ -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, ), ); @@ -67,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); } @@ -84,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); } @@ -99,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); } @@ -123,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); } @@ -141,15 +139,14 @@ 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 { 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/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); + } +} 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, ]; 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); + } +} 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/config/dependencies.config.php b/module/Core/config/dependencies.config.php index 567d57ce..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 [ @@ -38,6 +39,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, @@ -61,6 +63,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, @@ -69,6 +72,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, @@ -117,6 +124,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, @@ -137,6 +145,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'], @@ -161,7 +173,11 @@ return [ ], Action\RobotsAction::class => [Crawling\CrawlingHelper::class], - ShortUrl\Resolver\PersistenceShortUrlRelationResolver::class => ['em'], + 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/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/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/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/Resolver/PersistenceShortUrlRelationResolver.php b/module/Core/src/ShortUrl/Resolver/PersistenceShortUrlRelationResolver.php index db6721d5..17669f32 100644 --- a/module/Core/src/ShortUrl/Resolver/PersistenceShortUrlRelationResolver.php +++ b/module/Core/src/ShortUrl/Resolver/PersistenceShortUrlRelationResolver.php @@ -9,8 +9,13 @@ 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 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; @@ -20,31 +25,43 @@ 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) - { + 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); } public function resolveDomain(?string $domain): ?Domain { - if ($domain === null) { + if ($domain === null || $domain === $this->options->defaultDomain()) { 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 { - return $this->memoizedNewDomains[$domain] = $this->memoizedNewDomains[$domain] ?? Domain::withAuthority( - $domain, - ); + return $this->memoizedNewDomains[$domain] ??= Domain::withAuthority($domain); } /** @@ -61,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; @@ -71,12 +96,39 @@ 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); + } + + /** + * @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/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/src/ShortUrl/ShortUrlVisitsDeleter.php b/module/Core/src/ShortUrl/ShortUrlVisitsDeleter.php new file mode 100644 index 00000000..8ad6713f --- /dev/null +++ b/module/Core/src/ShortUrl/ShortUrlVisitsDeleter.php @@ -0,0 +1,29 @@ +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 new file mode 100644 index 00000000..46e9fde5 --- /dev/null +++ b/module/Core/src/ShortUrl/ShortUrlVisitsDeleterInterface.php @@ -0,0 +1,18 @@ +getEntityManager()->createQueryBuilder(); + $qb->delete(Visit::class, 'v') + ->where($qb->expr()->eq('v.shortUrl', ':shortUrl')) + ->setParameter('shortUrl', $shortUrl); + + 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 new file mode 100644 index 00000000..a1516225 --- /dev/null +++ b/module/Core/src/Visit/Repository/VisitDeleterRepositoryInterface.php @@ -0,0 +1,14 @@ +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 @@ +getEntityManager(); + $this->repo = new VisitDeleterRepository($em, $em->getClassMetadata(Visit::class)); + } + + #[Test] + public function deletesExpectedShortUrlVisits(): 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(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)); + } + + #[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/Core/test/ShortUrl/Resolver/PersistenceShortUrlRelationResolverTest.php b/module/Core/test/ShortUrl/Resolver/PersistenceShortUrlRelationResolverTest.php index 43f99462..d7af118d 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')] @@ -65,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; @@ -81,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()); } diff --git a/module/Core/test/ShortUrl/ShortUrlVisitsDeleterTest.php b/module/Core/test/ShortUrl/ShortUrlVisitsDeleterTest.php new file mode 100644 index 00000000..e1690a5b --- /dev/null +++ b/module/Core/test/ShortUrl/ShortUrlVisitsDeleterTest.php @@ -0,0 +1,55 @@ +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(''); + $shortUrl = ShortUrl::withLongUrl('https://example.com'); + + $this->resolver->expects($this->once())->method('resolveShortUrl')->with($identifier, null)->willReturn( + $shortUrl, + ); + $this->repository->expects($this->once())->method('deleteShortUrlVisits')->with($shortUrl)->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]; + } +} diff --git a/module/Core/test/Visit/VisitsDeleterTest.php b/module/Core/test/Visit/VisitsDeleterTest.php new file mode 100644 index 00000000..155d0725 --- /dev/null +++ b/module/Core/test/Visit/VisitsDeleterTest.php @@ -0,0 +1,41 @@ +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]; + } +} diff --git a/module/Rest/config/dependencies.config.php b/module/Rest/config/dependencies.config.php index cf394740..acca571d 100644 --- a/module/Rest/config/dependencies.config.php +++ b/module/Rest/config/dependencies.config.php @@ -32,11 +32,13 @@ return [ Action\ShortUrl\DeleteShortUrlAction::class => 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, 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, @@ -89,11 +91,13 @@ 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, 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')); + } +} 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/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, ) { } 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 { diff --git a/module/Rest/test-api/Action/CreateShortUrlTest.php b/module/Rest/test-api/Action/CreateShortUrlTest.php index 108a0f6f..5b22e79a 100644 --- a/module/Rest/test-api/Action/CreateShortUrlTest.php +++ b/module/Rest/test-api/Action/CreateShortUrlTest.php @@ -319,21 +319,6 @@ class CreateShortUrlTest extends ApiTestCase yield 'example domain' => ['example.com']; } - #[Test, DataProvider('provideTwitterUrls')] - public function urlsWithBothProtectionCanBeShortenedWithUrlValidationEnabled(string $longUrl): void - { - [$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 { 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 new file mode 100644 index 00000000..7b5f306d --- /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()); // 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 + { + $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()); + } +} 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]; + } +} 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]; + } +} 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 - +