diff --git a/Dockerfile b/Dockerfile index 2616323a..34d6d7ef 100644 --- a/Dockerfile +++ b/Dockerfile @@ -7,7 +7,6 @@ ENV SHLINK_RUNTIME ${SHLINK_RUNTIME} ARG SHLINK_USER_ID='root' ENV SHLINK_USER_ID ${SHLINK_USER_ID} -ENV OPENSWOOLE_VERSION 22.1.2 ENV PDO_SQLSRV_VERSION 5.12.0 ENV MS_ODBC_DOWNLOAD 'b/9/f/b9f3cce4-3925-46d4-9f46-da08869c6486' ENV MS_ODBC_SQL_VERSION 18_18.1.1.1 @@ -26,13 +25,8 @@ RUN \ apk del .dev-deps && \ apk add --no-cache postgresql icu libzip libpng -# Install openswoole and sqlsrv driver for x86_64 builds +# Install sqlsrv driver for x86_64 builds RUN apk add --no-cache --virtual .phpize-deps ${PHPIZE_DEPS} unixodbc-dev && \ - if [ "$SHLINK_RUNTIME" == 'openswoole' ]; then \ - # Openswoole is deprecated. Remove in v4.0.0 - pecl install openswoole-${OPENSWOOLE_VERSION} && \ - docker-php-ext-enable openswoole ; \ - fi; \ if [ $(uname -m) == "x86_64" ]; then \ wget https://download.microsoft.com/download/${MS_ODBC_DOWNLOAD}/msodbcsql${MS_ODBC_SQL_VERSION}-1_amd64.apk && \ apk add --allow-untrusted msodbcsql${MS_ODBC_SQL_VERSION}-1_amd64.apk && \ @@ -47,14 +41,7 @@ FROM base as builder COPY . . COPY --from=composer:2 /usr/bin/composer ./composer.phar RUN apk add --no-cache git && \ - # 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 \ - # Openswoole is deprecated. Remove in v4.0.0 - 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 install --no-dev --prefer-dist --optimize-autoloader --no-progress --no-interaction && \ php composer.phar clear-cache && \ rm -r docker composer.* && \ sed -i "s/%SHLINK_VERSION%/${SHLINK_VERSION}/g" config/autoload/app_options.global.php diff --git a/build.sh b/build.sh index f91ab408..7b77295f 100755 --- a/build.sh +++ b/build.sh @@ -1,18 +1,15 @@ #!/usr/bin/env bash set -e -if [ "$#" -lt 1 ] || [ "$#" -gt 2 ] || ([ "$#" == 2 ] && [ "$2" != "--no-swoole" ]); then +if [ "$#" -lt 1 ]; then echo "Usage:" >&2 - echo " $0 {version} [--no-swoole]" >&2 + echo " $0 {version}" >&2 exit 1 fi version=$1 -noSwoole=$2 phpVersion=$(php -r 'echo PHP_MAJOR_VERSION . "." . PHP_MINOR_VERSION;') -# Openswoole is deprecated. Remove in v4.0.0 -[[ $noSwoole ]] && swooleSuffix="" || swooleSuffix="_openswoole" -distId="shlink${version}_php${phpVersion}${swooleSuffix}_dist" +distId="shlink${version}_php${phpVersion}_dist" builtContent="./build/${distId}" projectdir=$(pwd) [[ -f ./composer.phar ]] && composerBin='./composer.phar' || composerBin='composer' @@ -31,18 +28,8 @@ cd "${builtContent}" # Install dependencies echo "Installing dependencies with $composerBin..." -composerFlags="--optimize-autoloader --no-progress --no-interaction" ${composerBin} self-update -${composerBin} install --no-dev --prefer-dist $composerFlags - -if [[ $noSwoole ]]; then - # If generating a dist not for openswoole, uninstall mezzio-swoole - ${composerBin} remove mezzio/mezzio-swoole --with-all-dependencies --update-no-dev $composerFlags -else - # Deprecated. Remove in Shlink v4.0.0 - # If generating a dist for openswoole, uninstall RoadRunner - ${composerBin} remove spiral/roadrunner spiral/roadrunner-jobs spiral/roadrunner-cli spiral/roadrunner-http --with-all-dependencies --update-no-dev $composerFlags -fi +${composerBin} install --no-dev --prefer-dist --optimize-autoloader --no-progress --no-interaction # Delete development files echo 'Deleting dev files...' diff --git a/config/autoload/installer.global.php b/config/autoload/installer.global.php index 45f92153..affb0897 100644 --- a/config/autoload/installer.global.php +++ b/config/autoload/installer.global.php @@ -21,8 +21,6 @@ return [ Option\Database\DatabaseUnixSocketConfigOption::class, Option\UrlShortener\ShortDomainHostConfigOption::class, Option\UrlShortener\ShortDomainSchemaConfigOption::class, - Option\Visit\VisitsWebhooksConfigOption::class, - Option\Visit\OrphanVisitsWebhooksConfigOption::class, Option\Redirect\BaseUrlRedirectConfigOption::class, Option\Redirect\InvalidShortUrlRedirectConfigOption::class, Option\Redirect\Regular404RedirectConfigOption::class, diff --git a/config/autoload/middleware-pipeline.global.php b/config/autoload/middleware-pipeline.global.php index 072937ec..99f71bce 100644 --- a/config/autoload/middleware-pipeline.global.php +++ b/config/autoload/middleware-pipeline.global.php @@ -47,7 +47,6 @@ return [ 'rest' => [ 'path' => '/rest', 'middleware' => [ - Rest\Middleware\ErrorHandler\BackwardsCompatibleProblemDetailsHandler::class, Router\Middleware\ImplicitOptionsMiddleware::class, Rest\Middleware\BodyParserMiddleware::class, Rest\Middleware\AuthenticationMiddleware::class, diff --git a/config/autoload/rabbit.global.php b/config/autoload/rabbit.global.php index bf9591e5..fd8cda68 100644 --- a/config/autoload/rabbit.global.php +++ b/config/autoload/rabbit.global.php @@ -14,9 +14,6 @@ return [ 'user' => EnvVars::RABBITMQ_USER->loadFromEnv(), 'password' => EnvVars::RABBITMQ_PASSWORD->loadFromEnv(), 'vhost' => EnvVars::RABBITMQ_VHOST->loadFromEnv('/'), - - // Deprecated - 'legacy_visits_publishing' => (bool) EnvVars::RABBITMQ_LEGACY_VISITS_PUBLISHING->loadFromEnv(false), ], ]; diff --git a/config/autoload/url-shortener.global.php b/config/autoload/url-shortener.global.php index 2a121bee..2816577d 100644 --- a/config/autoload/url-shortener.global.php +++ b/config/autoload/url-shortener.global.php @@ -14,7 +14,7 @@ return (static function (): array { MIN_SHORT_CODES_LENGTH, ); $modeFromEnv = EnvVars::SHORT_URL_MODE->loadFromEnv(ShortUrlMode::STRICT->value); - $mode = ShortUrlMode::tryDeprecated($modeFromEnv) ?? ShortUrlMode::STRICT; + $mode = ShortUrlMode::tryFrom($modeFromEnv) ?? ShortUrlMode::STRICT; return [ diff --git a/config/autoload/webhooks.global.php b/config/autoload/webhooks.global.php deleted file mode 100644 index e72c4904..00000000 --- a/config/autoload/webhooks.global.php +++ /dev/null @@ -1,20 +0,0 @@ -<?php - -declare(strict_types=1); - -use Shlinkio\Shlink\Core\Config\EnvVars; - -// Deprecated. Webhooks are no longer supported. To be removed in Shlink 4.0.0 -return (static function (): array { - $webhooks = EnvVars::VISITS_WEBHOOKS->loadFromEnv(); - - return [ - - 'visits_webhooks' => [ - 'webhooks' => $webhooks === null ? [] : explode(',', $webhooks), - 'notify_orphan_visits_to_webhooks' => - (bool) EnvVars::NOTIFY_ORPHAN_VISITS_TO_WEBHOOKS->loadFromEnv(false), - ], - - ]; -})(); diff --git a/config/constants.php b/config/constants.php index f08c135c..7b263262 100644 --- a/config/constants.php +++ b/config/constants.php @@ -9,7 +9,7 @@ use Shlinkio\Shlink\Core\Util\RedirectStatus; const DEFAULT_DELETE_SHORT_URL_THRESHOLD = 15; const DEFAULT_SHORT_CODES_LENGTH = 5; const MIN_SHORT_CODES_LENGTH = 4; -const DEFAULT_REDIRECT_STATUS_CODE = RedirectStatus::STATUS_302; // Deprecated. Default to 307 for Shlink v4 +const DEFAULT_REDIRECT_STATUS_CODE = RedirectStatus::STATUS_302; const DEFAULT_REDIRECT_CACHE_LIFETIME = 30; const LOCAL_LOCK_FACTORY = 'Shlinkio\Shlink\LocalLockFactory'; const TITLE_TAG_VALUE = '/<title[^>]*>(.*?)<\/title>/i'; // Matches the value inside a html title tag @@ -19,6 +19,5 @@ const DEFAULT_QR_CODE_MARGIN = 0; const DEFAULT_QR_CODE_FORMAT = 'png'; const DEFAULT_QR_CODE_ERROR_CORRECTION = 'l'; const DEFAULT_QR_CODE_ROUND_BLOCK_SIZE = true; -// Deprecated. Shlink 4.0.0 should change default value to `true` -const DEFAULT_QR_CODE_ENABLED_FOR_DISABLED_SHORT_URLS = false; +const DEFAULT_QR_CODE_ENABLED_FOR_DISABLED_SHORT_URLS = true; const MIN_TASK_WORKERS = 4; diff --git a/docker/docker-entrypoint.sh b/docker/docker-entrypoint.sh index 6c95bee2..faa506a9 100644 --- a/docker/docker-entrypoint.sh +++ b/docker/docker-entrypoint.sh @@ -20,19 +20,6 @@ fi php vendor/bin/shlink-installer init ${flags} -# Periodically run visit:locate every hour, if ENABLE_PERIODIC_VISIT_LOCATE=true was provided and running as root -# FIXME: 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 & -fi - -if [ "$SHLINK_RUNTIME" = 'openswoole' ]; then - # Openswoole is deprecated. Remove in Shlink 4.0.0 - # When restarting the container, openswoole might think it is already in execution - # This forces the app to be started every second until the exit code is 0 - until php vendor/bin/laminas mezzio:swoole:start; do sleep 1 ; done -elif [ "$SHLINK_RUNTIME" = 'rr' ]; then +if [ "$SHLINK_RUNTIME" = 'rr' ]; then ./bin/rr serve -c config/roadrunner/.rr.yml fi diff --git a/docs/async-api/async-api.json b/docs/async-api/async-api.json index d45dae2b..d3177b98 100644 --- a/docs/async-api/async-api.json +++ b/docs/async-api/async-api.json @@ -122,11 +122,6 @@ "visitsSummary": { "$ref": "#/components/schemas/VisitsSummary" }, - "visitsCount": { - "deprecated": true, - "type": "integer", - "description": "The number of visits that this short URL has received." - }, "tags": { "type": "array", "items": { diff --git a/docs/swagger/definitions/ShortUrl.json b/docs/swagger/definitions/ShortUrl.json index 98fd9c87..8a420e9b 100644 --- a/docs/swagger/definitions/ShortUrl.json +++ b/docs/swagger/definitions/ShortUrl.json @@ -6,7 +6,6 @@ "longUrl", "deviceLongUrls", "dateCreated", - "visitsCount", "visitsSummary", "tags", "meta", @@ -36,11 +35,6 @@ "format": "date-time", "description": "The date in which the short URL was created in ISO format." }, - "visitsCount": { - "deprecated": true, - "type": "integer", - "description": "**[DEPRECATED]** Use `visitsSummary.total` instead." - }, "visitsSummary": { "$ref": "./VisitsSummary.json" }, diff --git a/docs/swagger/definitions/TagInfo.json b/docs/swagger/definitions/TagInfo.json index 41de1068..27658c95 100644 --- a/docs/swagger/definitions/TagInfo.json +++ b/docs/swagger/definitions/TagInfo.json @@ -1,6 +1,6 @@ { "type": "object", - "required": ["tag", "shortUrlsCount", "visitsSummary", "visitsCount"], + "required": ["tag", "shortUrlsCount", "visitsSummary"], "properties": { "tag": { "type": "string", @@ -12,11 +12,6 @@ }, "visitsSummary": { "$ref": "./VisitsSummary.json" - }, - "visitsCount": { - "deprecated": true, - "type": "number", - "description": "**[DEPRECATED]** Use visitsSummary.total instead" } } } diff --git a/docs/swagger/definitions/VisitStats.json b/docs/swagger/definitions/VisitStats.json index 2ed24375..a1d8ce19 100644 --- a/docs/swagger/definitions/VisitStats.json +++ b/docs/swagger/definitions/VisitStats.json @@ -1,22 +1,12 @@ { "type": "object", - "required": ["nonOrphanVisits", "orphanVisits", "visitsCount", "orphanVisitsCount"], + "required": ["nonOrphanVisits", "orphanVisits"], "properties": { "nonOrphanVisits": { "$ref": "./VisitsSummary.json" }, "orphanVisits": { "$ref": "./VisitsSummary.json" - }, - "visitsCount": { - "deprecated": true, - "type": "number", - "description": "**[DEPRECATED]** Use nonOrphanVisits.total instead" - }, - "orphanVisitsCount": { - "deprecated": true, - "type": "number", - "description": "**[DEPRECATED]** Use orphanVisits.total instead" } } } diff --git a/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php b/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php index c9497daf..297c435e 100644 --- a/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php +++ b/module/CLI/src/Command/ShortUrl/ListShortUrlsCommand.php @@ -218,7 +218,7 @@ class ListShortUrlsCommand extends Command 'Short URL' => $pickProp('shortUrl'), 'Long URL' => $pickProp('longUrl'), 'Date created' => $pickProp('dateCreated'), - 'Visits count' => $pickProp('visitsCount'), + 'Visits count' => static fn (array $shortUrl) => $shortUrl['visitsSummary']->total, ]; if ($input->getOption('show-tags')) { $columnsMap['Tags'] = static fn (array $shortUrl): string => implode(', ', $shortUrl['tags']); diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index 591fcc79..6b6be190 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -31,7 +31,6 @@ return [ Options\TrackingOptions::class => [ValinorConfigFactory::class, 'config.tracking'], Options\QrCodeOptions::class => [ValinorConfigFactory::class, 'config.qr_codes'], Options\RabbitMqOptions::class => [ValinorConfigFactory::class, 'config.rabbitmq'], - Options\WebhookOptions::class => ConfigAbstractFactory::class, ShortUrl\UrlShortener::class => ConfigAbstractFactory::class, ShortUrl\ShortUrlService::class => ConfigAbstractFactory::class, @@ -113,8 +112,6 @@ return [ Domain\DomainService::class, ], - Options\WebhookOptions::class => ['config.visits_webhooks'], - ShortUrl\UrlShortener::class => [ ShortUrl\Helper\ShortUrlTitleResolutionHelper::class, 'em', diff --git a/module/Core/config/event_dispatcher.config.php b/module/Core/config/event_dispatcher.config.php index 1a81d8ed..012b8e12 100644 --- a/module/Core/config/event_dispatcher.config.php +++ b/module/Core/config/event_dispatcher.config.php @@ -37,7 +37,6 @@ return (static function (): array { EventDispatcher\Mercure\NotifyVisitToMercure::class, EventDispatcher\RabbitMq\NotifyVisitToRabbitMq::class, EventDispatcher\RedisPubSub\NotifyVisitToRedis::class, - EventDispatcher\NotifyVisitToWebHooks::class, EventDispatcher\UpdateGeoLiteDb::class, ], EventDispatcher\Event\ShortUrlCreated::class => [ @@ -66,7 +65,6 @@ return (static function (): array { EventDispatcher\LocateVisit::class => ConfigAbstractFactory::class, EventDispatcher\Matomo\SendVisitToMatomo::class => ConfigAbstractFactory::class, EventDispatcher\LocateUnlocatedVisits::class => ConfigAbstractFactory::class, - EventDispatcher\NotifyVisitToWebHooks::class => ConfigAbstractFactory::class, EventDispatcher\Mercure\NotifyVisitToMercure::class => ConfigAbstractFactory::class, EventDispatcher\Mercure\NotifyNewShortUrlToMercure::class => ConfigAbstractFactory::class, EventDispatcher\RabbitMq\NotifyVisitToRabbitMq::class => ConfigAbstractFactory::class, @@ -104,9 +102,6 @@ return (static function (): array { EventDispatcher\LocateUnlocatedVisits::class => [ EventDispatcher\CloseDbConnectionEventListenerDelegator::class, ], - EventDispatcher\NotifyVisitToWebHooks::class => [ - EventDispatcher\CloseDbConnectionEventListenerDelegator::class, - ], ], ], @@ -119,14 +114,6 @@ return (static function (): array { EventDispatcherInterface::class, ], EventDispatcher\LocateUnlocatedVisits::class => [VisitLocator::class, VisitToLocationHelper::class], - EventDispatcher\NotifyVisitToWebHooks::class => [ - 'httpClient', - 'em', - 'Logger_Shlink', - Options\WebhookOptions::class, - ShortUrl\Transformer\ShortUrlDataTransformer::class, - Options\AppOptions::class, - ], EventDispatcher\Mercure\NotifyVisitToMercure::class => [ MercureHubPublishingHelper::class, EventDispatcher\PublishingUpdatesGenerator::class, @@ -144,7 +131,6 @@ return (static function (): array { EventDispatcher\PublishingUpdatesGenerator::class, 'em', 'Logger_Shlink', - Visit\Transformer\OrphanVisitDataTransformer::class, Options\RabbitMqOptions::class, ], EventDispatcher\RabbitMq\NotifyNewShortUrlToRabbitMq::class => [ @@ -187,7 +173,6 @@ return (static function (): array { Options\RabbitMqOptions::class, 'config.redis.pub_sub_enabled', MercureOptions::class, - Options\WebhookOptions::class, GeoLite2Options::class, MatomoOptions::class, ], diff --git a/module/Core/src/Config/EnvVars.php b/module/Core/src/Config/EnvVars.php index 40f311e9..4a32b7c3 100644 --- a/module/Core/src/Config/EnvVars.php +++ b/module/Core/src/Config/EnvVars.php @@ -36,8 +36,6 @@ enum EnvVars: string case RABBITMQ_PASSWORD = 'RABBITMQ_PASSWORD'; case RABBITMQ_VHOST = 'RABBITMQ_VHOST'; case RABBITMQ_USE_SSL = 'RABBITMQ_USE_SSL'; - /** @deprecated */ - case RABBITMQ_LEGACY_VISITS_PUBLISHING = 'RABBITMQ_LEGACY_VISITS_PUBLISHING'; case MATOMO_ENABLED = 'MATOMO_ENABLED'; case MATOMO_BASE_URL = 'MATOMO_BASE_URL'; case MATOMO_SITE_ID = 'MATOMO_SITE_ID'; @@ -74,10 +72,6 @@ enum EnvVars: string case REDIRECT_APPEND_EXTRA_PATH = 'REDIRECT_APPEND_EXTRA_PATH'; case TIMEZONE = 'TIMEZONE'; case MULTI_SEGMENT_SLUGS_ENABLED = 'MULTI_SEGMENT_SLUGS_ENABLED'; - /** @deprecated */ - case VISITS_WEBHOOKS = 'VISITS_WEBHOOKS'; - /** @deprecated */ - case NOTIFY_ORPHAN_VISITS_TO_WEBHOOKS = 'NOTIFY_ORPHAN_VISITS_TO_WEBHOOKS'; public function loadFromEnv(mixed $default = null): mixed { diff --git a/module/Core/src/EventDispatcher/Helper/EnabledListenerChecker.php b/module/Core/src/EventDispatcher/Helper/EnabledListenerChecker.php index 269aed76..ad4c8070 100644 --- a/module/Core/src/EventDispatcher/Helper/EnabledListenerChecker.php +++ b/module/Core/src/EventDispatcher/Helper/EnabledListenerChecker.php @@ -8,19 +8,17 @@ use Shlinkio\Shlink\Common\Mercure\MercureOptions; use Shlinkio\Shlink\Core\EventDispatcher; use Shlinkio\Shlink\Core\Matomo\MatomoOptions; use Shlinkio\Shlink\Core\Options\RabbitMqOptions; -use Shlinkio\Shlink\Core\Options\WebhookOptions; use Shlinkio\Shlink\EventDispatcher\Listener\EnabledListenerCheckerInterface; use Shlinkio\Shlink\IpGeolocation\GeoLite2\GeoLite2Options; -class EnabledListenerChecker implements EnabledListenerCheckerInterface +readonly class EnabledListenerChecker implements EnabledListenerCheckerInterface { public function __construct( - private readonly RabbitMqOptions $rabbitMqOptions, - private readonly bool $redisPubSubEnabled, - private readonly MercureOptions $mercureOptions, - private readonly WebhookOptions $webhookOptions, - private readonly GeoLite2Options $geoLiteOptions, - private readonly MatomoOptions $matomoOptions, + private RabbitMqOptions $rabbitMqOptions, + private bool $redisPubSubEnabled, + private MercureOptions $mercureOptions, + private GeoLite2Options $geoLiteOptions, + private MatomoOptions $matomoOptions, ) { } @@ -38,7 +36,6 @@ class EnabledListenerChecker implements EnabledListenerCheckerInterface EventDispatcher\Mercure\NotifyVisitToMercure::class, EventDispatcher\Mercure\NotifyNewShortUrlToMercure::class => $this->mercureOptions->isEnabled(), EventDispatcher\Matomo\SendVisitToMatomo::class => $this->matomoOptions->enabled, - EventDispatcher\NotifyVisitToWebHooks::class => $this->webhookOptions->hasWebhooks(), EventDispatcher\UpdateGeoLiteDb::class => $this->geoLiteOptions->hasLicenseKey(), default => false, // Any unknown async listener should not be enabled by default }; diff --git a/module/Core/src/EventDispatcher/NotifyVisitToWebHooks.php b/module/Core/src/EventDispatcher/NotifyVisitToWebHooks.php deleted file mode 100644 index 028c3c13..00000000 --- a/module/Core/src/EventDispatcher/NotifyVisitToWebHooks.php +++ /dev/null @@ -1,101 +0,0 @@ -<?php - -declare(strict_types=1); - -namespace Shlinkio\Shlink\Core\EventDispatcher; - -use Doctrine\ORM\EntityManagerInterface; -use Fig\Http\Message\RequestMethodInterface; -use GuzzleHttp\ClientInterface; -use GuzzleHttp\Promise\Promise; -use GuzzleHttp\Promise\PromiseInterface; -use GuzzleHttp\Promise\Utils; -use GuzzleHttp\RequestOptions; -use Psr\Log\LoggerInterface; -use Shlinkio\Shlink\Common\Rest\DataTransformerInterface; -use Shlinkio\Shlink\Core\EventDispatcher\Event\VisitLocated; -use Shlinkio\Shlink\Core\Options\AppOptions; -use Shlinkio\Shlink\Core\Options\WebhookOptions; -use Shlinkio\Shlink\Core\Visit\Entity\Visit; -use Throwable; - -use function array_map; - -/** @deprecated */ -class NotifyVisitToWebHooks -{ - public function __construct( - private readonly ClientInterface $httpClient, - private readonly EntityManagerInterface $em, - private readonly LoggerInterface $logger, - private readonly WebhookOptions $webhookOptions, - private readonly DataTransformerInterface $transformer, - private readonly AppOptions $appOptions, - ) { - } - - public function __invoke(VisitLocated $shortUrlLocated): void - { - if (! $this->webhookOptions->hasWebhooks()) { - return; - } - - $visitId = $shortUrlLocated->visitId; - - /** @var Visit|null $visit */ - $visit = $this->em->find(Visit::class, $visitId); - if ($visit === null) { - $this->logger->warning('Tried to notify webhooks for visit with id "{visitId}", but it does not exist.', [ - 'visitId' => $visitId, - ]); - return; - } - - if ($visit->isOrphan() && ! $this->webhookOptions->notifyOrphanVisits()) { - return; - } - - $requestOptions = $this->buildRequestOptions($visit); - $requestPromises = $this->performRequests($requestOptions, $visitId); - - // Wait for all the promises to finish, ignoring rejections, as in those cases we only want to log the error. - Utils::settle($requestPromises)->wait(); - } - - private function buildRequestOptions(Visit $visit): array - { - $payload = ['visit' => $visit->jsonSerialize()]; - $shortUrl = $visit->getShortUrl(); - if ($shortUrl !== null) { - $payload['shortUrl'] = $this->transformer->transform($shortUrl); - } - - return [ - RequestOptions::TIMEOUT => 10, - RequestOptions::JSON => $payload, - RequestOptions::HEADERS => ['User-Agent' => $this->appOptions->__toString()], - ]; - } - - /** - * @param Promise[] $requestOptions - */ - private function performRequests(array $requestOptions, string $visitId): array - { - return array_map( - fn (string $webhook): PromiseInterface => $this->httpClient - ->requestAsync(RequestMethodInterface::METHOD_POST, $webhook, $requestOptions) - ->otherwise(fn (Throwable $e) => $this->logWebhookFailure($webhook, $visitId, $e)), - $this->webhookOptions->webhooks(), - ); - } - - private function logWebhookFailure(string $webhook, string $visitId, Throwable $e): void - { - $this->logger->warning('Failed to notify visit with id "{visitId}" to webhook "{webhook}". {e}', [ - 'visitId' => $visitId, - 'webhook' => $webhook, - 'e' => $e, - ]); - } -} diff --git a/module/Core/src/EventDispatcher/RabbitMq/NotifyVisitToRabbitMq.php b/module/Core/src/EventDispatcher/RabbitMq/NotifyVisitToRabbitMq.php index ed5b08e0..ddc4221c 100644 --- a/module/Core/src/EventDispatcher/RabbitMq/NotifyVisitToRabbitMq.php +++ b/module/Core/src/EventDispatcher/RabbitMq/NotifyVisitToRabbitMq.php @@ -6,15 +6,11 @@ namespace Shlinkio\Shlink\Core\EventDispatcher\RabbitMq; use Doctrine\ORM\EntityManagerInterface; use Psr\Log\LoggerInterface; -use Shlinkio\Shlink\Common\Rest\DataTransformerInterface; use Shlinkio\Shlink\Common\UpdatePublishing\PublishingHelperInterface; -use Shlinkio\Shlink\Common\UpdatePublishing\Update; use Shlinkio\Shlink\Core\EventDispatcher\Async\AbstractNotifyVisitListener; use Shlinkio\Shlink\Core\EventDispatcher\Async\RemoteSystem; use Shlinkio\Shlink\Core\EventDispatcher\PublishingUpdatesGeneratorInterface; -use Shlinkio\Shlink\Core\EventDispatcher\Topic; use Shlinkio\Shlink\Core\Options\RabbitMqOptions; -use Shlinkio\Shlink\Core\Visit\Entity\Visit; class NotifyVisitToRabbitMq extends AbstractNotifyVisitListener { @@ -23,42 +19,11 @@ class NotifyVisitToRabbitMq extends AbstractNotifyVisitListener PublishingUpdatesGeneratorInterface $updatesGenerator, EntityManagerInterface $em, LoggerInterface $logger, - private readonly DataTransformerInterface $orphanVisitTransformer, private readonly RabbitMqOptions $options, ) { parent::__construct($rabbitMqHelper, $updatesGenerator, $em, $logger); } - /** - * @return Update[] - */ - protected function determineUpdatesForVisit(Visit $visit): array - { - // Once the two deprecated cases below have been removed, make parent method private - if (! $this->options->legacyVisitsPublishing) { - return parent::determineUpdatesForVisit($visit); - } - - // This was defined incorrectly. - // According to the spec, both the visit and the short URL it belongs to, should be published. - // The shape should be ['visit' => [...], 'shortUrl' => ?[...]] - // However, this would be a breaking change, so we need a flag that determines the shape of the payload. - return $visit->isOrphan() - ? [ - Update::forTopicAndPayload( - Topic::NEW_ORPHAN_VISIT->value, - $this->orphanVisitTransformer->transform($visit), - ), - ] - : [ - Update::forTopicAndPayload(Topic::NEW_VISIT->value, $visit->jsonSerialize()), - Update::forTopicAndPayload( - Topic::newShortUrlVisit($visit->getShortUrl()?->getShortCode()), - $visit->jsonSerialize(), - ), - ]; - } - protected function isEnabled(): bool { return $this->options->enabled; diff --git a/module/Core/src/Options/RabbitMqOptions.php b/module/Core/src/Options/RabbitMqOptions.php index cc25f3bf..308dff2a 100644 --- a/module/Core/src/Options/RabbitMqOptions.php +++ b/module/Core/src/Options/RabbitMqOptions.php @@ -4,12 +4,10 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Options; -final class RabbitMqOptions +final readonly class RabbitMqOptions { public function __construct( - public readonly bool $enabled = false, - /** @deprecated */ - public readonly bool $legacyVisitsPublishing = false, + public bool $enabled = false, ) { } } diff --git a/module/Core/src/Options/WebhookOptions.php b/module/Core/src/Options/WebhookOptions.php deleted file mode 100644 index 7196fd0c..00000000 --- a/module/Core/src/Options/WebhookOptions.php +++ /dev/null @@ -1,41 +0,0 @@ -<?php - -declare(strict_types=1); - -namespace Shlinkio\Shlink\Core\Options; - -use Laminas\Stdlib\AbstractOptions; - -/** @deprecated */ -class WebhookOptions extends AbstractOptions -{ - protected $__strictMode__ = false; // phpcs:ignore - - private array $webhooks = []; - private bool $notifyOrphanVisitsToWebhooks = false; - - public function webhooks(): array - { - return $this->webhooks; - } - - public function hasWebhooks(): bool - { - return ! empty($this->webhooks); - } - - protected function setWebhooks(array $webhooks): void - { - $this->webhooks = $webhooks; - } - - public function notifyOrphanVisits(): bool - { - return $this->notifyOrphanVisitsToWebhooks; - } - - protected function setNotifyOrphanVisitsToWebhooks(bool $notifyOrphanVisitsToWebhooks): void - { - $this->notifyOrphanVisitsToWebhooks = $notifyOrphanVisitsToWebhooks; - } -} diff --git a/module/Core/src/ShortUrl/Model/ShortUrlMode.php b/module/Core/src/ShortUrl/Model/ShortUrlMode.php index d359e8cc..19886657 100644 --- a/module/Core/src/ShortUrl/Model/ShortUrlMode.php +++ b/module/Core/src/ShortUrl/Model/ShortUrlMode.php @@ -6,10 +6,4 @@ enum ShortUrlMode: string { case STRICT = 'strict'; case LOOSE = 'loose'; - - /** @deprecated */ - public static function tryDeprecated(string $mode): ?self - { - return $mode === 'loosely' ? self::LOOSE : self::tryFrom($mode); - } } diff --git a/module/Core/src/ShortUrl/Transformer/ShortUrlDataTransformer.php b/module/Core/src/ShortUrl/Transformer/ShortUrlDataTransformer.php index a6641998..d0661504 100644 --- a/module/Core/src/ShortUrl/Transformer/ShortUrlDataTransformer.php +++ b/module/Core/src/ShortUrl/Transformer/ShortUrlDataTransformer.php @@ -39,9 +39,6 @@ class ShortUrlDataTransformer implements DataTransformerInterface $shortUrl->getVisitsCount(), $shortUrl->nonBotVisitsCount(), ), - - // Deprecated - 'visitsCount' => $shortUrl->getVisitsCount(), ]; } diff --git a/module/Core/src/Tag/Model/OrderableField.php b/module/Core/src/Tag/Model/OrderableField.php index b7a9509f..39092e4d 100644 --- a/module/Core/src/Tag/Model/OrderableField.php +++ b/module/Core/src/Tag/Model/OrderableField.php @@ -10,15 +10,13 @@ enum OrderableField: string case SHORT_URLS_COUNT = 'shortUrlsCount'; case VISITS = 'visits'; case NON_BOT_VISITS = 'nonBotVisits'; - /** @deprecated Use VISITS instead */ - case VISITS_COUNT = 'visitsCount'; - public static function toSnakeCaseValidField(?string $field): self + public static function toValidField(?string $field): self { - $parsed = $field !== null ? self::tryFrom($field) : self::TAG; - return match ($parsed) { - self::VISITS_COUNT, null => self::VISITS, - default => $parsed, - }; + if ($field === null) { + return self::TAG; + } + + return self::tryFrom($field) ?? self::TAG; } } diff --git a/module/Core/src/Tag/Model/TagInfo.php b/module/Core/src/Tag/Model/TagInfo.php index 4c0018b2..504181ec 100644 --- a/module/Core/src/Tag/Model/TagInfo.php +++ b/module/Core/src/Tag/Model/TagInfo.php @@ -7,13 +7,13 @@ namespace Shlinkio\Shlink\Core\Tag\Model; use JsonSerializable; use Shlinkio\Shlink\Core\Visit\Model\VisitsSummary; -final class TagInfo implements JsonSerializable +final readonly class TagInfo implements JsonSerializable { - public readonly VisitsSummary $visitsSummary; + public VisitsSummary $visitsSummary; public function __construct( - public readonly string $tag, - public readonly int $shortUrlsCount, + public string $tag, + public int $shortUrlsCount, int $visitsCount, ?int $nonBotVisitsCount = null, ) { @@ -36,9 +36,6 @@ final class TagInfo implements JsonSerializable 'tag' => $this->tag, 'shortUrlsCount' => $this->shortUrlsCount, 'visitsSummary' => $this->visitsSummary, - - // Deprecated - 'visitsCount' => $this->visitsSummary->total, ]; } } diff --git a/module/Core/src/Tag/Model/TagsParams.php b/module/Core/src/Tag/Model/TagsParams.php index 3b1d84b2..422f9da1 100644 --- a/module/Core/src/Tag/Model/TagsParams.php +++ b/module/Core/src/Tag/Model/TagsParams.php @@ -14,8 +14,6 @@ final class TagsParams extends AbstractInfinitePaginableListParams private function __construct( public readonly ?string $searchTerm, public readonly Ordering $orderBy, - /** @deprecated */ - public readonly bool $withStats, ?int $page, ?int $itemsPerPage, ) { @@ -27,7 +25,6 @@ final class TagsParams extends AbstractInfinitePaginableListParams return new self( $query['searchTerm'] ?? null, Ordering::fromTuple(isset($query['orderBy']) ? parseOrderBy($query['orderBy']) : [null, null]), - ($query['withStats'] ?? null) === 'true', isset($query['page']) ? (int) $query['page'] : null, isset($query['itemsPerPage']) ? (int) $query['itemsPerPage'] : null, ); diff --git a/module/Core/src/Tag/Repository/TagRepository.php b/module/Core/src/Tag/Repository/TagRepository.php index ce8b1f76..7f07e867 100644 --- a/module/Core/src/Tag/Repository/TagRepository.php +++ b/module/Core/src/Tag/Repository/TagRepository.php @@ -43,7 +43,7 @@ class TagRepository extends EntitySpecificationRepository implements TagReposito */ public function findTagsWithInfo(?TagsListFiltering $filtering = null): array { - $orderField = OrderableField::toSnakeCaseValidField($filtering?->orderBy?->field); + $orderField = OrderableField::toValidField($filtering?->orderBy?->field); $orderDir = $filtering?->orderBy?->direction ?? 'ASC'; $apiKey = $filtering?->apiKey; $conn = $this->getEntityManager()->getConnection(); diff --git a/module/Core/src/Visit/Model/VisitsStats.php b/module/Core/src/Visit/Model/VisitsStats.php index adac34eb..22f05bd4 100644 --- a/module/Core/src/Visit/Model/VisitsStats.php +++ b/module/Core/src/Visit/Model/VisitsStats.php @@ -6,10 +6,10 @@ namespace Shlinkio\Shlink\Core\Visit\Model; use JsonSerializable; -final class VisitsStats implements JsonSerializable +final readonly class VisitsStats implements JsonSerializable { - private readonly VisitsSummary $nonOrphanVisitsSummary; - private readonly VisitsSummary $orphanVisitsSummary; + private VisitsSummary $nonOrphanVisitsSummary; + private VisitsSummary $orphanVisitsSummary; public function __construct( int $nonOrphanVisitsTotal, @@ -32,10 +32,6 @@ final class VisitsStats implements JsonSerializable return [ 'nonOrphanVisits' => $this->nonOrphanVisitsSummary, 'orphanVisits' => $this->orphanVisitsSummary, - - // Deprecated - 'visitsCount' => $this->nonOrphanVisitsSummary->total, - 'orphanVisitsCount' => $this->orphanVisitsSummary->total, ]; } } diff --git a/module/Core/test-api/Action/QrCodeTest.php b/module/Core/test-api/Action/QrCodeTest.php index 955e6c7e..21fd5147 100644 --- a/module/Core/test-api/Action/QrCodeTest.php +++ b/module/Core/test-api/Action/QrCodeTest.php @@ -10,7 +10,7 @@ use Shlinkio\Shlink\TestUtils\ApiTest\ApiTestCase; class QrCodeTest extends ApiTestCase { #[Test] - public function returnsNotFoundWhenShortUrlIsNotEnabled(): void + public function returnsQrCodeEvenIfShortUrlIsNotEnabled(): void { // The QR code successfully resolves at first $response = $this->callShortUrl('custom/qr-code'); @@ -20,8 +20,8 @@ class QrCodeTest extends ApiTestCase $this->callShortUrl('custom'); $this->callShortUrl('custom'); - // After 2 visits, the QR code should return a 404 - $response = $this->callShortUrl('custom/qr-code'); - self::assertEquals(404, $response->getStatusCode()); + // After 2 visits, the short URL returns a 404, but the QR code should still work + self::assertEquals(404, $this->callShortUrl('custom')->getStatusCode()); + self::assertEquals(200, $this->callShortUrl('custom/qr-code')->getStatusCode()); } } diff --git a/module/Core/test-db/Tag/Repository/TagRepositoryTest.php b/module/Core/test-db/Tag/Repository/TagRepositoryTest.php index 6cccf199..77077142 100644 --- a/module/Core/test-db/Tag/Repository/TagRepositoryTest.php +++ b/module/Core/test-db/Tag/Repository/TagRepositoryTest.php @@ -197,13 +197,6 @@ class TagRepositoryTest extends DatabaseTestCase ['another', 0, 0, 0], ], ]; - yield 'visits count DESC ordering and limit' => [ - new TagsListFiltering(2, null, null, Ordering::fromTuple([OrderableField::VISITS_COUNT->value, 'DESC'])), - [ - ['foo', 2, 4, 3], - ['bar', 3, 3, 2], - ], - ]; yield 'api key' => [new TagsListFiltering(null, null, null, null, ApiKey::fromMeta( ApiKeyMeta::withRoles(RoleDefinition::forAuthoredShortUrls()), )), [ diff --git a/module/Core/test/Action/QrCodeActionTest.php b/module/Core/test/Action/QrCodeActionTest.php index 9a89ff47..98e1e375 100644 --- a/module/Core/test/Action/QrCodeActionTest.php +++ b/module/Core/test/Action/QrCodeActionTest.php @@ -265,7 +265,7 @@ class QrCodeActionTest extends TestCase $this->urlResolver, new ShortUrlStringifier(['domain' => 's.test']), new NullLogger(), - $options ?? new QrCodeOptions(), + $options ?? new QrCodeOptions(enabledForDisabledShortUrls: false), ); } } diff --git a/module/Core/test/EventDispatcher/Helper/EnabledListenerCheckerTest.php b/module/Core/test/EventDispatcher/Helper/EnabledListenerCheckerTest.php index 00f78fe4..cebde437 100644 --- a/module/Core/test/EventDispatcher/Helper/EnabledListenerCheckerTest.php +++ b/module/Core/test/EventDispatcher/Helper/EnabledListenerCheckerTest.php @@ -12,7 +12,6 @@ use Shlinkio\Shlink\Core\EventDispatcher\Helper\EnabledListenerChecker; use Shlinkio\Shlink\Core\EventDispatcher\Matomo\SendVisitToMatomo; use Shlinkio\Shlink\Core\EventDispatcher\Mercure\NotifyNewShortUrlToMercure; use Shlinkio\Shlink\Core\EventDispatcher\Mercure\NotifyVisitToMercure; -use Shlinkio\Shlink\Core\EventDispatcher\NotifyVisitToWebHooks; use Shlinkio\Shlink\Core\EventDispatcher\RabbitMq\NotifyNewShortUrlToRabbitMq; use Shlinkio\Shlink\Core\EventDispatcher\RabbitMq\NotifyVisitToRabbitMq; use Shlinkio\Shlink\Core\EventDispatcher\RedisPubSub\NotifyNewShortUrlToRedis; @@ -20,7 +19,6 @@ use Shlinkio\Shlink\Core\EventDispatcher\RedisPubSub\NotifyVisitToRedis; use Shlinkio\Shlink\Core\EventDispatcher\UpdateGeoLiteDb; use Shlinkio\Shlink\Core\Matomo\MatomoOptions; use Shlinkio\Shlink\Core\Options\RabbitMqOptions; -use Shlinkio\Shlink\Core\Options\WebhookOptions; use Shlinkio\Shlink\IpGeolocation\GeoLite2\GeoLite2Options; class EnabledListenerCheckerTest extends TestCase @@ -41,7 +39,6 @@ class EnabledListenerCheckerTest extends TestCase [NotifyVisitToMercure::class], [NotifyNewShortUrlToMercure::class], [SendVisitToMatomo::class], - [NotifyVisitToWebHooks::class], [UpdateGeoLiteDb::class], ]; } @@ -68,7 +65,6 @@ class EnabledListenerCheckerTest extends TestCase NotifyNewShortUrlToRedis::class => false, NotifyVisitToMercure::class => false, NotifyNewShortUrlToMercure::class => false, - NotifyVisitToWebHooks::class => false, UpdateGeoLiteDb::class => false, 'unknown' => false, ]]; @@ -79,7 +75,6 @@ class EnabledListenerCheckerTest extends TestCase NotifyNewShortUrlToRedis::class => true, NotifyVisitToMercure::class => false, NotifyNewShortUrlToMercure::class => false, - NotifyVisitToWebHooks::class => false, UpdateGeoLiteDb::class => false, 'unknown' => false, ]]; @@ -90,18 +85,6 @@ class EnabledListenerCheckerTest extends TestCase NotifyNewShortUrlToRedis::class => false, NotifyVisitToMercure::class => true, NotifyNewShortUrlToMercure::class => true, - NotifyVisitToWebHooks::class => false, - UpdateGeoLiteDb::class => false, - 'unknown' => false, - ]]; - yield 'Webhooks' => [self::checker(webhooksEnabled: true), [ - NotifyVisitToRabbitMq::class => false, - NotifyNewShortUrlToRabbitMq::class => false, - NotifyVisitToRedis::class => false, - NotifyNewShortUrlToRedis::class => false, - NotifyVisitToMercure::class => false, - NotifyNewShortUrlToMercure::class => false, - NotifyVisitToWebHooks::class => true, UpdateGeoLiteDb::class => false, 'unknown' => false, ]]; @@ -112,7 +95,6 @@ class EnabledListenerCheckerTest extends TestCase NotifyNewShortUrlToRedis::class => false, NotifyVisitToMercure::class => false, NotifyNewShortUrlToMercure::class => false, - NotifyVisitToWebHooks::class => false, UpdateGeoLiteDb::class => true, 'unknown' => false, ]]; @@ -124,7 +106,6 @@ class EnabledListenerCheckerTest extends TestCase NotifyVisitToMercure::class => false, NotifyNewShortUrlToMercure::class => false, SendVisitToMatomo::class => true, - NotifyVisitToWebHooks::class => false, UpdateGeoLiteDb::class => false, 'unknown' => false, ]]; @@ -135,7 +116,6 @@ class EnabledListenerCheckerTest extends TestCase NotifyNewShortUrlToRedis::class => false, NotifyVisitToMercure::class => false, NotifyNewShortUrlToMercure::class => false, - NotifyVisitToWebHooks::class => false, UpdateGeoLiteDb::class => false, 'unknown' => false, ]]; @@ -143,7 +123,6 @@ class EnabledListenerCheckerTest extends TestCase rabbitMqEnabled: true, redisPubSubEnabled: true, mercureEnabled: true, - webhooksEnabled: true, geoLiteEnabled: true, matomoEnabled: true, ), [ @@ -154,7 +133,6 @@ class EnabledListenerCheckerTest extends TestCase NotifyVisitToMercure::class => true, NotifyNewShortUrlToMercure::class => true, SendVisitToMatomo::class => true, - NotifyVisitToWebHooks::class => true, UpdateGeoLiteDb::class => true, 'unknown' => false, ]]; @@ -164,7 +142,6 @@ class EnabledListenerCheckerTest extends TestCase bool $rabbitMqEnabled = false, bool $redisPubSubEnabled = false, bool $mercureEnabled = false, - bool $webhooksEnabled = false, bool $geoLiteEnabled = false, bool $matomoEnabled = false, ): EnabledListenerChecker { @@ -172,7 +149,6 @@ class EnabledListenerCheckerTest extends TestCase new RabbitMqOptions(enabled: $rabbitMqEnabled), $redisPubSubEnabled, new MercureOptions(publicHubUrl: $mercureEnabled ? 'the-url' : null), - new WebhookOptions(['webhooks' => $webhooksEnabled ? ['foo', 'bar'] : []]), new GeoLite2Options(licenseKey: $geoLiteEnabled ? 'the-key' : null), new MatomoOptions(enabled: $matomoEnabled), ); diff --git a/module/Core/test/EventDispatcher/NotifyVisitToWebHooksTest.php b/module/Core/test/EventDispatcher/NotifyVisitToWebHooksTest.php deleted file mode 100644 index 8b9c10ac..00000000 --- a/module/Core/test/EventDispatcher/NotifyVisitToWebHooksTest.php +++ /dev/null @@ -1,144 +0,0 @@ -<?php - -declare(strict_types=1); - -namespace ShlinkioTest\Shlink\Core\EventDispatcher; - -use Doctrine\ORM\EntityManagerInterface; -use Exception; -use Fig\Http\Message\RequestMethodInterface; -use GuzzleHttp\ClientInterface; -use GuzzleHttp\Promise\FulfilledPromise; -use GuzzleHttp\Promise\RejectedPromise; -use GuzzleHttp\RequestOptions; -use PHPUnit\Framework\Assert; -use PHPUnit\Framework\Attributes\DataProvider; -use PHPUnit\Framework\Attributes\Test; -use PHPUnit\Framework\MockObject\MockObject; -use PHPUnit\Framework\TestCase; -use Psr\Log\LoggerInterface; -use Shlinkio\Shlink\Core\EventDispatcher\Event\VisitLocated; -use Shlinkio\Shlink\Core\EventDispatcher\NotifyVisitToWebHooks; -use Shlinkio\Shlink\Core\Options\AppOptions; -use Shlinkio\Shlink\Core\Options\WebhookOptions; -use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; -use Shlinkio\Shlink\Core\ShortUrl\Helper\ShortUrlStringifier; -use Shlinkio\Shlink\Core\ShortUrl\Transformer\ShortUrlDataTransformer; -use Shlinkio\Shlink\Core\Visit\Entity\Visit; -use Shlinkio\Shlink\Core\Visit\Model\Visitor; - -use function count; -use function Shlinkio\Shlink\Core\ArrayUtils\contains; - -class NotifyVisitToWebHooksTest extends TestCase -{ - private MockObject & ClientInterface $httpClient; - private MockObject & EntityManagerInterface $em; - private MockObject & LoggerInterface $logger; - - protected function setUp(): void - { - $this->httpClient = $this->createMock(ClientInterface::class); - $this->em = $this->createMock(EntityManagerInterface::class); - $this->logger = $this->createMock(LoggerInterface::class); - } - - #[Test] - public function emptyWebhooksMakeNoFurtherActions(): void - { - $this->em->expects($this->never())->method('find'); - - $this->createListener([])(new VisitLocated('1')); - } - - #[Test] - public function invalidVisitDoesNotPerformAnyRequest(): void - { - $this->em->expects($this->once())->method('find')->with(Visit::class, '1')->willReturn(null); - $this->httpClient->expects($this->never())->method('requestAsync'); - $this->logger->expects($this->once())->method('warning')->with( - 'Tried to notify webhooks for visit with id "{visitId}", but it does not exist.', - ['visitId' => '1'], - ); - - $this->createListener(['foo', 'bar'])(new VisitLocated('1')); - } - - #[Test] - public function orphanVisitDoesNotPerformAnyRequestWhenDisabled(): void - { - $this->em->expects($this->once())->method('find')->with(Visit::class, '1')->willReturn( - Visit::forBasePath(Visitor::emptyInstance()), - ); - $this->httpClient->expects($this->never())->method('requestAsync'); - $this->logger->expects($this->never())->method('warning'); - - $this->createListener(['foo', 'bar'], false)(new VisitLocated('1')); - } - - #[Test, DataProvider('provideVisits')] - public function expectedRequestsArePerformedToWebhooks(Visit $visit, array $expectedResponseKeys): void - { - $webhooks = ['foo', 'invalid', 'bar', 'baz']; - $invalidWebhooks = ['invalid', 'baz']; - - $this->em->expects($this->once())->method('find')->with(Visit::class, '1')->willReturn($visit); - $this->httpClient->expects($this->exactly(count($webhooks)))->method('requestAsync')->with( - RequestMethodInterface::METHOD_POST, - $this->istype('string'), - $this->callback(function (array $requestOptions) use ($expectedResponseKeys) { - Assert::assertArrayHasKey(RequestOptions::HEADERS, $requestOptions); - Assert::assertArrayHasKey(RequestOptions::JSON, $requestOptions); - Assert::assertArrayHasKey(RequestOptions::TIMEOUT, $requestOptions); - Assert::assertEquals(10, $requestOptions[RequestOptions::TIMEOUT]); - Assert::assertEquals(['User-Agent' => 'Shlink:v1.2.3'], $requestOptions[RequestOptions::HEADERS]); - - $json = $requestOptions[RequestOptions::JSON]; - Assert::assertCount(count($expectedResponseKeys), $json); - foreach ($expectedResponseKeys as $key) { - Assert::assertArrayHasKey($key, $json); - } - - return true; - }), - )->willReturnCallback(function ($_, $webhook) use ($invalidWebhooks) { - $shouldReject = contains($webhook, $invalidWebhooks); - return $shouldReject ? new RejectedPromise(new Exception('')) : new FulfilledPromise(''); - }); - $this->logger->expects($this->exactly(count($invalidWebhooks)))->method('warning')->with( - 'Failed to notify visit with id "{visitId}" to webhook "{webhook}". {e}', - $this->callback(function (array $extra): bool { - Assert::assertArrayHasKey('webhook', $extra); - Assert::assertArrayHasKey('visitId', $extra); - Assert::assertArrayHasKey('e', $extra); - - return true; - }), - ); - - $this->createListener($webhooks)(new VisitLocated('1')); - } - - public static function provideVisits(): iterable - { - yield 'regular visit' => [ - Visit::forValidShortUrl(ShortUrl::createFake(), Visitor::emptyInstance()), - ['shortUrl', 'visit'], - ]; - yield 'orphan visit' => [Visit::forBasePath(Visitor::emptyInstance()), ['visit']]; - } - - private function createListener(array $webhooks, bool $notifyOrphanVisits = true): NotifyVisitToWebHooks - { - return new NotifyVisitToWebHooks( - $this->httpClient, - $this->em, - $this->logger, - new WebhookOptions( - ['webhooks' => $webhooks, 'notify_orphan_visits_to_webhooks' => $notifyOrphanVisits], - ), - new ShortUrlDataTransformer(new ShortUrlStringifier([])), - new AppOptions('Shlink', '1.2.3'), - ); - } -} diff --git a/module/Core/test/EventDispatcher/PublishingUpdatesGeneratorTest.php b/module/Core/test/EventDispatcher/PublishingUpdatesGeneratorTest.php index 9d28f2cd..c5ebb1a8 100644 --- a/module/Core/test/EventDispatcher/PublishingUpdatesGeneratorTest.php +++ b/module/Core/test/EventDispatcher/PublishingUpdatesGeneratorTest.php @@ -53,7 +53,6 @@ class PublishingUpdatesGeneratorTest extends TestCase 'longUrl' => 'https://longUrl', 'deviceLongUrls' => $shortUrl->deviceLongUrls(), 'dateCreated' => $shortUrl->getDateCreated()->toAtomString(), - 'visitsCount' => 0, 'tags' => [], 'meta' => [ 'validSince' => null, @@ -128,7 +127,6 @@ class PublishingUpdatesGeneratorTest extends TestCase 'longUrl' => 'https://longUrl', 'deviceLongUrls' => $shortUrl->deviceLongUrls(), 'dateCreated' => $shortUrl->getDateCreated()->toAtomString(), - 'visitsCount' => 0, 'tags' => [], 'meta' => [ 'validSince' => null, diff --git a/module/Core/test/EventDispatcher/RabbitMq/NotifyVisitToRabbitMqTest.php b/module/Core/test/EventDispatcher/RabbitMq/NotifyVisitToRabbitMqTest.php index e722bf25..7386169f 100644 --- a/module/Core/test/EventDispatcher/RabbitMq/NotifyVisitToRabbitMqTest.php +++ b/module/Core/test/EventDispatcher/RabbitMq/NotifyVisitToRabbitMqTest.php @@ -7,7 +7,6 @@ namespace ShlinkioTest\Shlink\Core\EventDispatcher\RabbitMq; use Doctrine\ORM\EntityManagerInterface; use DomainException; use Exception; -use PHPUnit\Framework\Assert; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\MockObject\MockObject; @@ -24,7 +23,6 @@ use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlCreation; use Shlinkio\Shlink\Core\Visit\Entity\Visit; use Shlinkio\Shlink\Core\Visit\Model\Visitor; -use Shlinkio\Shlink\Core\Visit\Transformer\OrphanVisitDataTransformer; use Throwable; use function array_walk; @@ -132,9 +130,8 @@ class NotifyVisitToRabbitMqTest extends TestCase yield [new DomainException('DomainException Error')]; } - #[Test, DataProvider('provideLegacyPayloads')] + #[Test, DataProvider('providePayloads')] public function expectedPayloadIsPublishedDependingOnConfig( - bool $legacy, Visit $visit, callable $setup, callable $expect, @@ -144,44 +141,12 @@ class NotifyVisitToRabbitMqTest extends TestCase $setup($this->updatesGenerator); $expect($this->helper, $this->updatesGenerator); - ($this->listener(new RabbitMqOptions(true, $legacy)))(new VisitLocated($visitId)); + ($this->listener())(new VisitLocated($visitId)); } - public static function provideLegacyPayloads(): iterable + public static function providePayloads(): iterable { - yield 'legacy non-orphan visit' => [ - true, - $visit = Visit::forValidShortUrl(ShortUrl::withLongUrl('https://longUrl'), Visitor::emptyInstance()), - static fn () => null, - function (MockObject & PublishingHelperInterface $helper) use ($visit): void { - $helper->method('publishUpdate')->with(self::callback(function (Update $update) use ($visit): bool { - $payload = $update->payload; - Assert::assertEquals($payload, $visit->jsonSerialize()); - Assert::assertArrayNotHasKey('visitedUrl', $payload); - Assert::assertArrayNotHasKey('type', $payload); - Assert::assertArrayNotHasKey('visit', $payload); - Assert::assertArrayNotHasKey('shortUrl', $payload); - - return true; - })); - }, - ]; - yield 'legacy orphan visit' => [ - true, - Visit::forBasePath(Visitor::emptyInstance()), - static fn () => null, - function (MockObject & PublishingHelperInterface $helper): void { - $helper->method('publishUpdate')->with(self::callback(function (Update $update): bool { - $payload = $update->payload; - Assert::assertArrayHasKey('visitedUrl', $payload); - Assert::assertArrayHasKey('type', $payload); - - return true; - })); - }, - ]; - yield 'non-legacy non-orphan visit' => [ - false, + yield 'non-orphan visit' => [ Visit::forValidShortUrl(ShortUrl::withLongUrl('https://longUrl'), Visitor::emptyInstance()), function (MockObject & PublishingUpdatesGeneratorInterface $updatesGenerator): void { $update = Update::forTopicAndPayload('', []); @@ -195,8 +160,7 @@ class NotifyVisitToRabbitMqTest extends TestCase $helper->expects(self::exactly(2))->method('publishUpdate')->with(self::isInstanceOf(Update::class)); }, ]; - yield 'non-legacy orphan visit' => [ - false, + yield 'orphan visit' => [ Visit::forBasePath(Visitor::emptyInstance()), function (MockObject & PublishingUpdatesGeneratorInterface $updatesGenerator): void { $update = Update::forTopicAndPayload('', []); @@ -217,8 +181,7 @@ class NotifyVisitToRabbitMqTest extends TestCase $this->updatesGenerator, $this->em, $this->logger, - new OrphanVisitDataTransformer(), - $options ?? new RabbitMqOptions(enabled: true, legacyVisitsPublishing: false), + $options ?? new RabbitMqOptions(enabled: true), ); } } diff --git a/module/Core/test/ShortUrl/Model/ShortUrlModeTest.php b/module/Core/test/ShortUrl/Model/ShortUrlModeTest.php deleted file mode 100644 index f2ca7cce..00000000 --- a/module/Core/test/ShortUrl/Model/ShortUrlModeTest.php +++ /dev/null @@ -1,28 +0,0 @@ -<?php - -declare(strict_types=1); - -namespace ShlinkioTest\Shlink\Core\ShortUrl\Model; - -use PHPUnit\Framework\Attributes\DataProvider; -use PHPUnit\Framework\Attributes\Test; -use PHPUnit\Framework\TestCase; -use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlMode; - -class ShortUrlModeTest extends TestCase -{ - #[Test, DataProvider('provideModes')] - public function deprecatedValuesAreProperlyParsed(string $mode, ?ShortUrlMode $expected): void - { - self::assertSame($expected, ShortUrlMode::tryDeprecated($mode)); - } - - public static function provideModes(): iterable - { - yield 'invalid' => ['invalid', null]; - yield 'foo' => ['foo', null]; - yield 'loose' => ['loose', ShortUrlMode::LOOSE]; - yield 'loosely' => ['loosely', ShortUrlMode::LOOSE]; - yield 'strict' => ['strict', ShortUrlMode::STRICT]; - } -} diff --git a/module/Rest/config/dependencies.config.php b/module/Rest/config/dependencies.config.php index acca571d..67343e27 100644 --- a/module/Rest/config/dependencies.config.php +++ b/module/Rest/config/dependencies.config.php @@ -55,7 +55,6 @@ return [ Middleware\ShortUrl\DefaultShortCodesLengthMiddleware::class => ConfigAbstractFactory::class, Middleware\ShortUrl\OverrideDomainMiddleware::class => ConfigAbstractFactory::class, Middleware\Mercure\NotConfiguredMercureErrorHandler::class => ConfigAbstractFactory::class, - Middleware\ErrorHandler\BackwardsCompatibleProblemDetailsHandler::class => InvokableFactory::class, ], ], diff --git a/module/Rest/src/Action/Tag/ListTagsAction.php b/module/Rest/src/Action/Tag/ListTagsAction.php index 9674d5bc..13898584 100644 --- a/module/Rest/src/Action/Tag/ListTagsAction.php +++ b/module/Rest/src/Action/Tag/ListTagsAction.php @@ -8,14 +8,11 @@ use Laminas\Diactoros\Response\JsonResponse; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; use Shlinkio\Shlink\Common\Paginator\Util\PagerfantaUtilsTrait; -use Shlinkio\Shlink\Core\Tag\Model\TagInfo; use Shlinkio\Shlink\Core\Tag\Model\TagsParams; use Shlinkio\Shlink\Core\Tag\TagServiceInterface; use Shlinkio\Shlink\Rest\Action\AbstractRestAction; use Shlinkio\Shlink\Rest\Middleware\AuthenticationMiddleware; -use function array_map; - class ListTagsAction extends AbstractRestAction { use PagerfantaUtilsTrait; @@ -32,17 +29,8 @@ class ListTagsAction extends AbstractRestAction $params = TagsParams::fromRawData($request->getQueryParams()); $apiKey = AuthenticationMiddleware::apiKeyFromRequest($request); - if (! $params->withStats) { - return new JsonResponse([ - 'tags' => $this->serializePaginator($this->tagService->listTags($params, $apiKey)), - ]); - } - - // This part is deprecated. To get tags with stats, the /tags/stats endpoint should be used instead - $tagsInfo = $this->tagService->tagsInfo($params, $apiKey); - $rawTags = $this->serializePaginator($tagsInfo, dataProp: 'stats'); - $rawTags['data'] = array_map(static fn (TagInfo $info) => $info->tag, [...$tagsInfo]); - - return new JsonResponse(['tags' => $rawTags]); + return new JsonResponse([ + 'tags' => $this->serializePaginator($this->tagService->listTags($params, $apiKey)), + ]); } } diff --git a/module/Rest/src/ApiKey/Role.php b/module/Rest/src/ApiKey/Role.php index dd2d8ae7..4f3685db 100644 --- a/module/Rest/src/ApiKey/Role.php +++ b/module/Rest/src/ApiKey/Role.php @@ -40,8 +40,8 @@ enum Role: string public static function toSpec(ApiKeyRole $role, ?string $context = null): Specification { - return match ($role->role()) { - self::AUTHORED_SHORT_URLS => new BelongsToApiKey($role->apiKey(), $context), + return match ($role->role) { + self::AUTHORED_SHORT_URLS => new BelongsToApiKey($role->apiKey, $context), self::DOMAIN_SPECIFIC => new BelongsToDomain(self::domainIdFromMeta($role->meta()), $context), default => Spec::andX(), }; @@ -49,8 +49,8 @@ enum Role: string public static function toInlinedSpec(ApiKeyRole $role): Specification { - return match ($role->role()) { - self::AUTHORED_SHORT_URLS => Spec::andX(new BelongsToApiKeyInlined($role->apiKey())), + return match ($role->role) { + self::AUTHORED_SHORT_URLS => Spec::andX(new BelongsToApiKeyInlined($role->apiKey)), self::DOMAIN_SPECIFIC => Spec::andX(new BelongsToDomainInlined(self::domainIdFromMeta($role->meta()))), default => Spec::andX(), }; diff --git a/module/Rest/src/Entity/ApiKey.php b/module/Rest/src/Entity/ApiKey.php index dae30de0..9ad3fcf4 100644 --- a/module/Rest/src/Entity/ApiKey.php +++ b/module/Rest/src/Entity/ApiKey.php @@ -156,7 +156,7 @@ class ApiKey extends AbstractEntity */ public function mapRoles(callable $fun): array { - return $this->roles->map(fn (ApiKeyRole $role) => $fun($role->role(), $role->meta()))->getValues(); + return $this->roles->map(fn (ApiKeyRole $role) => $fun($role->role, $role->meta()))->getValues(); } public function registerRole(RoleDefinition $roleDefinition): void diff --git a/module/Rest/src/Entity/ApiKeyRole.php b/module/Rest/src/Entity/ApiKeyRole.php index 6fadb839..5053b74d 100644 --- a/module/Rest/src/Entity/ApiKeyRole.php +++ b/module/Rest/src/Entity/ApiKeyRole.php @@ -13,22 +13,6 @@ class ApiKeyRole extends AbstractEntity { } - /** - * @deprecated Use property access directly - */ - public function role(): Role - { - return $this->role; - } - - /** - * @deprecated Use property access directly - */ - public function apiKey(): ApiKey - { - return $this->apiKey; - } - public function meta(): array { return $this->meta; diff --git a/module/Rest/src/Exception/BackwardsCompatibleProblemDetailsException.php b/module/Rest/src/Exception/BackwardsCompatibleProblemDetailsException.php deleted file mode 100644 index 8cfb918c..00000000 --- a/module/Rest/src/Exception/BackwardsCompatibleProblemDetailsException.php +++ /dev/null @@ -1,99 +0,0 @@ -<?php - -declare(strict_types=1); - -namespace Shlinkio\Shlink\Rest\Exception; - -use Mezzio\ProblemDetails\Exception\ProblemDetailsExceptionInterface; -use Shlinkio\Shlink\Core\Exception\DeleteShortUrlException; -use Shlinkio\Shlink\Core\Exception\DomainNotFoundException; -use Shlinkio\Shlink\Core\Exception\ForbiddenTagOperationException; -use Shlinkio\Shlink\Core\Exception\InvalidUrlException; -use Shlinkio\Shlink\Core\Exception\NonUniqueSlugException; -use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; -use Shlinkio\Shlink\Core\Exception\TagConflictException; -use Shlinkio\Shlink\Core\Exception\TagNotFoundException; -use Shlinkio\Shlink\Core\Exception\ValidationException; - -use function end; -use function explode; - -/** @deprecated */ -class BackwardsCompatibleProblemDetailsException extends RuntimeException implements ProblemDetailsExceptionInterface -{ - private function __construct(private readonly ProblemDetailsExceptionInterface $e) - { - parent::__construct($e->getMessage(), $e->getCode(), $e); - } - - public static function fromProblemDetails(ProblemDetailsExceptionInterface $e): self - { - return new self($e); - } - - public function getStatus(): int - { - return $this->e->getStatus(); - } - - public function getType(): string - { - return $this->remapType($this->e->getType()); - } - - public function getTitle(): string - { - return $this->e->getTitle(); - } - - public function getDetail(): string - { - return $this->e->getDetail(); - } - - public function getAdditionalData(): array - { - return $this->e->getAdditionalData(); - } - - public function toArray(): array - { - return $this->remapTypeInArray($this->e->toArray()); - } - - public function jsonSerialize(): array - { - return $this->remapTypeInArray($this->e->jsonSerialize()); - } - - private function remapTypeInArray(array $wrappedArray): array - { - if (! isset($wrappedArray['type'])) { - return $wrappedArray; - } - - return [...$wrappedArray, 'type' => $this->remapType($wrappedArray['type'])]; - } - - private function remapType(string $wrappedType): string - { - $segments = explode('/', $wrappedType); - $lastSegment = end($segments); - - return match ($lastSegment) { - ValidationException::ERROR_CODE => 'INVALID_ARGUMENT', - DeleteShortUrlException::ERROR_CODE => 'INVALID_SHORT_URL_DELETION', - DomainNotFoundException::ERROR_CODE => 'DOMAIN_NOT_FOUND', - ForbiddenTagOperationException::ERROR_CODE => 'FORBIDDEN_OPERATION', - InvalidUrlException::ERROR_CODE => 'INVALID_URL', - NonUniqueSlugException::ERROR_CODE => 'INVALID_SLUG', - ShortUrlNotFoundException::ERROR_CODE => 'INVALID_SHORTCODE', - TagConflictException::ERROR_CODE => 'TAG_CONFLICT', - TagNotFoundException::ERROR_CODE => 'TAG_NOT_FOUND', - MercureException::ERROR_CODE => 'MERCURE_NOT_CONFIGURED', - MissingAuthenticationException::ERROR_CODE => 'INVALID_AUTHORIZATION', - VerifyAuthenticationException::ERROR_CODE => 'INVALID_API_KEY', - default => $wrappedType, - }; - } -} diff --git a/module/Rest/src/Middleware/ErrorHandler/BackwardsCompatibleProblemDetailsHandler.php b/module/Rest/src/Middleware/ErrorHandler/BackwardsCompatibleProblemDetailsHandler.php deleted file mode 100644 index c099ad70..00000000 --- a/module/Rest/src/Middleware/ErrorHandler/BackwardsCompatibleProblemDetailsHandler.php +++ /dev/null @@ -1,30 +0,0 @@ -<?php - -declare(strict_types=1); - -namespace Shlinkio\Shlink\Rest\Middleware\ErrorHandler; - -use Mezzio\ProblemDetails\Exception\ProblemDetailsExceptionInterface; -use Psr\Http\Message\ResponseInterface; -use Psr\Http\Message\ServerRequestInterface; -use Psr\Http\Server\MiddlewareInterface; -use Psr\Http\Server\RequestHandlerInterface; -use Shlinkio\Shlink\Rest\Exception\BackwardsCompatibleProblemDetailsException; - -use function version_compare; - -/** @deprecated */ -class BackwardsCompatibleProblemDetailsHandler implements MiddlewareInterface -{ - public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface - { - try { - return $handler->handle($request); - } catch (ProblemDetailsExceptionInterface $e) { - $version = $request->getAttribute('version') ?? '2'; - throw version_compare($version, '3', '>=') - ? $e - : BackwardsCompatibleProblemDetailsException::fromProblemDetails($e); - } - } -} diff --git a/module/Rest/test-api/Action/CreateShortUrlTest.php b/module/Rest/test-api/Action/CreateShortUrlTest.php index 01592129..efd70666 100644 --- a/module/Rest/test-api/Action/CreateShortUrlTest.php +++ b/module/Rest/test-api/Action/CreateShortUrlTest.php @@ -19,7 +19,7 @@ class CreateShortUrlTest extends ApiTestCase #[Test] public function createsNewShortUrlWhenOnlyLongUrlIsProvided(): void { - $expectedKeys = ['shortCode', 'shortUrl', 'longUrl', 'dateCreated', 'visitsCount', 'tags']; + $expectedKeys = ['shortCode', 'shortUrl', 'longUrl', 'dateCreated', 'tags']; [$statusCode, $payload] = $this->createShortUrl(); self::assertEquals(self::STATUS_OK, $statusCode); @@ -48,7 +48,7 @@ class CreateShortUrlTest extends ApiTestCase self::assertEquals(self::STATUS_BAD_REQUEST, $statusCode); self::assertEquals(self::STATUS_BAD_REQUEST, $payload['status']); self::assertEquals($detail, $payload['detail']); - self::assertEquals('INVALID_SLUG', $payload['type']); + self::assertEquals('https://shlink.io/api/error/non-unique-slug', $payload['type']); self::assertEquals('Invalid custom slug', $payload['title']); self::assertEquals($slug, $payload['customSlug']); @@ -70,8 +70,8 @@ class CreateShortUrlTest extends ApiTestCase public static function provideDuplicatedSlugApiVersions(): iterable { - yield ['1', 'INVALID_SLUG']; - yield ['2', 'INVALID_SLUG']; + yield ['1', 'https://shlink.io/api/error/non-unique-slug']; + yield ['2', 'https://shlink.io/api/error/non-unique-slug']; yield ['3', 'https://shlink.io/api/error/non-unique-slug']; } @@ -241,7 +241,7 @@ class CreateShortUrlTest extends ApiTestCase public static function provideInvalidUrls(): iterable { - yield 'API version 2' => ['https://this-has-to-be-invalid.com', '2', 'INVALID_URL']; + yield 'API version 2' => ['https://this-has-to-be-invalid.com', '2', 'https://shlink.io/api/error/invalid-url']; yield 'API version 3' => ['https://this-has-to-be-invalid.com', '3', 'https://shlink.io/api/error/invalid-url']; } @@ -264,18 +264,18 @@ class CreateShortUrlTest extends ApiTestCase public static function provideInvalidArgumentApiVersions(): iterable { - yield 'missing long url v2' => [[], '2', 'INVALID_ARGUMENT']; + yield 'missing long url v2' => [[], '2', 'https://shlink.io/api/error/invalid-data']; yield 'missing long url v3' => [[], '3', 'https://shlink.io/api/error/invalid-data']; - yield 'empty long url v2' => [['longUrl' => null], '2', 'INVALID_ARGUMENT']; + yield 'empty long url v2' => [['longUrl' => null], '2', 'https://shlink.io/api/error/invalid-data']; yield 'empty long url v3' => [['longUrl' => ' '], '3', 'https://shlink.io/api/error/invalid-data']; - yield 'missing url schema v2' => [['longUrl' => 'foo.com'], '2', 'INVALID_ARGUMENT']; + yield 'missing url schema v2' => [['longUrl' => 'foo.com'], '2', 'https://shlink.io/api/error/invalid-data']; yield 'missing url schema v3' => [['longUrl' => 'foo.com'], '3', 'https://shlink.io/api/error/invalid-data']; yield 'empty device long url v2' => [[ 'longUrl' => 'foo', 'deviceLongUrls' => [ 'android' => null, ], - ], '2', 'INVALID_ARGUMENT']; + ], '2', 'https://shlink.io/api/error/invalid-data']; yield 'empty device long url v3' => [[ 'longUrl' => 'foo', 'deviceLongUrls' => [ diff --git a/module/Rest/test-api/Action/DeleteShortUrlTest.php b/module/Rest/test-api/Action/DeleteShortUrlTest.php index 7bd3dfea..06848c48 100644 --- a/module/Rest/test-api/Action/DeleteShortUrlTest.php +++ b/module/Rest/test-api/Action/DeleteShortUrlTest.php @@ -31,7 +31,7 @@ class DeleteShortUrlTest extends ApiTestCase self::assertEquals(self::STATUS_NOT_FOUND, $resp->getStatusCode()); self::assertEquals(self::STATUS_NOT_FOUND, $payload['status']); - self::assertEquals('INVALID_SHORTCODE', $payload['type']); + self::assertEquals('https://shlink.io/api/error/short-url-not-found', $payload['type']); self::assertEquals($expectedDetail, $payload['detail']); self::assertEquals('Short URL not found', $payload['title']); self::assertEquals($shortCode, $payload['shortCode']); @@ -52,8 +52,8 @@ class DeleteShortUrlTest extends ApiTestCase public static function provideApiVersions(): iterable { - yield ['1', 'INVALID_SHORTCODE']; - yield ['2', 'INVALID_SHORTCODE']; + yield ['1', 'https://shlink.io/api/error/short-url-not-found']; + yield ['2', 'https://shlink.io/api/error/short-url-not-found']; yield ['3', 'https://shlink.io/api/error/short-url-not-found']; } diff --git a/module/Rest/test-api/Action/DeleteTagsTest.php b/module/Rest/test-api/Action/DeleteTagsTest.php index b04fbaf5..a269d2db 100644 --- a/module/Rest/test-api/Action/DeleteTagsTest.php +++ b/module/Rest/test-api/Action/DeleteTagsTest.php @@ -30,8 +30,8 @@ class DeleteTagsTest extends ApiTestCase public static function provideNonAdminApiKeys(): iterable { - yield 'author' => ['author_api_key', '2', 'FORBIDDEN_OPERATION']; - yield 'domain' => ['domain_api_key', '2', 'FORBIDDEN_OPERATION']; + yield 'author' => ['author_api_key', '2', 'https://shlink.io/api/error/forbidden-tag-operation']; + yield 'domain' => ['domain_api_key', '2', 'https://shlink.io/api/error/forbidden-tag-operation']; yield 'version 3' => ['domain_api_key', '3', 'https://shlink.io/api/error/forbidden-tag-operation']; } } diff --git a/module/Rest/test-api/Action/DomainRedirectsTest.php b/module/Rest/test-api/Action/DomainRedirectsTest.php index bc78d035..d97092d6 100644 --- a/module/Rest/test-api/Action/DomainRedirectsTest.php +++ b/module/Rest/test-api/Action/DomainRedirectsTest.php @@ -21,7 +21,7 @@ class DomainRedirectsTest extends ApiTestCase self::assertEquals(self::STATUS_BAD_REQUEST, $resp->getStatusCode()); self::assertEquals(self::STATUS_BAD_REQUEST, $payload['status']); - self::assertEquals('INVALID_ARGUMENT', $payload['type']); + self::assertEquals('https://shlink.io/api/error/invalid-data', $payload['type']); self::assertEquals('Provided data is not valid', $payload['detail']); self::assertEquals('Invalid data', $payload['title']); } diff --git a/module/Rest/test-api/Action/DomainVisitsTest.php b/module/Rest/test-api/Action/DomainVisitsTest.php index 2c1d1d2e..3a06257b 100644 --- a/module/Rest/test-api/Action/DomainVisitsTest.php +++ b/module/Rest/test-api/Action/DomainVisitsTest.php @@ -49,7 +49,7 @@ class DomainVisitsTest extends ApiTestCase self::assertEquals(self::STATUS_NOT_FOUND, $resp->getStatusCode()); self::assertEquals(self::STATUS_NOT_FOUND, $payload['status']); - self::assertEquals('DOMAIN_NOT_FOUND', $payload['type']); + self::assertEquals('https://shlink.io/api/error/domain-not-found', $payload['type']); self::assertEquals(sprintf('Domain with authority "%s" could not be found', $domain), $payload['detail']); self::assertEquals('Domain not found', $payload['title']); self::assertEquals($domain, $payload['authority']); @@ -73,8 +73,8 @@ class DomainVisitsTest extends ApiTestCase public static function provideApiVersions(): iterable { - yield ['1', 'DOMAIN_NOT_FOUND']; - yield ['2', 'DOMAIN_NOT_FOUND']; + yield ['1', 'https://shlink.io/api/error/domain-not-found']; + yield ['2', 'https://shlink.io/api/error/domain-not-found']; yield ['3', 'https://shlink.io/api/error/domain-not-found']; } } diff --git a/module/Rest/test-api/Action/EditShortUrlTest.php b/module/Rest/test-api/Action/EditShortUrlTest.php index a55fb066..89055adb 100644 --- a/module/Rest/test-api/Action/EditShortUrlTest.php +++ b/module/Rest/test-api/Action/EditShortUrlTest.php @@ -96,7 +96,7 @@ class EditShortUrlTest extends ApiTestCase public static function provideLongUrls(): iterable { yield 'valid URL' => ['https://shlink.io', self::STATUS_OK, null]; - yield 'invalid URL' => ['http://foo', self::STATUS_BAD_REQUEST, 'INVALID_URL']; + yield 'invalid URL' => ['http://foo', self::STATUS_BAD_REQUEST, 'https://shlink.io/api/error/invalid-url']; } #[Test, DataProviderExternal(ApiTestDataProviders::class, 'invalidUrlsProvider')] @@ -112,7 +112,7 @@ class EditShortUrlTest extends ApiTestCase self::assertEquals(self::STATUS_NOT_FOUND, $resp->getStatusCode()); self::assertEquals(self::STATUS_NOT_FOUND, $payload['status']); - self::assertEquals('INVALID_SHORTCODE', $payload['type']); + self::assertEquals('https://shlink.io/api/error/short-url-not-found', $payload['type']); self::assertEquals($expectedDetail, $payload['detail']); self::assertEquals('Short URL not found', $payload['title']); self::assertEquals($shortCode, $payload['shortCode']); @@ -131,7 +131,7 @@ class EditShortUrlTest extends ApiTestCase self::assertEquals(self::STATUS_BAD_REQUEST, $resp->getStatusCode()); self::assertEquals(self::STATUS_BAD_REQUEST, $payload['status']); - self::assertEquals('INVALID_ARGUMENT', $payload['type']); + self::assertEquals('https://shlink.io/api/error/invalid-data', $payload['type']); self::assertEquals($expectedDetail, $payload['detail']); self::assertEquals('Invalid data', $payload['title']); } diff --git a/module/Rest/test-api/Action/GlobalVisitsTest.php b/module/Rest/test-api/Action/GlobalVisitsTest.php index 657f16a6..30c880d5 100644 --- a/module/Rest/test-api/Action/GlobalVisitsTest.php +++ b/module/Rest/test-api/Action/GlobalVisitsTest.php @@ -17,10 +17,8 @@ class GlobalVisitsTest extends ApiTestCase $payload = $this->getJsonResponsePayload($resp); self::assertArrayHasKey('visits', $payload); - self::assertArrayHasKey('visitsCount', $payload['visits']); - self::assertArrayHasKey('orphanVisitsCount', $payload['visits']); - self::assertEquals($expectedVisits, $payload['visits']['visitsCount']); - self::assertEquals($expectedOrphanVisits, $payload['visits']['orphanVisitsCount']); + self::assertEquals($expectedVisits, $payload['visits']['nonOrphanVisits']['total']); + self::assertEquals($expectedOrphanVisits, $payload['visits']['orphanVisits']['total']); } public static function provideApiKeys(): iterable diff --git a/module/Rest/test-api/Action/ListShortUrlsTest.php b/module/Rest/test-api/Action/ListShortUrlsTest.php index bb6296f7..3591ea60 100644 --- a/module/Rest/test-api/Action/ListShortUrlsTest.php +++ b/module/Rest/test-api/Action/ListShortUrlsTest.php @@ -20,7 +20,6 @@ class ListShortUrlsTest extends ApiTestCase 'shortUrl' => 'http://s.test/abc123', 'longUrl' => 'https://shlink.io', 'dateCreated' => '2018-05-01T00:00:00+00:00', - 'visitsCount' => 3, 'visitsSummary' => [ 'total' => 3, 'nonBots' => 3, @@ -42,7 +41,6 @@ class ListShortUrlsTest extends ApiTestCase 'shortUrl' => 'http://s.test/ghi789', 'longUrl' => 'https://shlink.io/documentation/', 'dateCreated' => '2018-05-01T00:00:00+00:00', - 'visitsCount' => 2, 'visitsSummary' => [ 'total' => 2, 'nonBots' => 2, @@ -64,7 +62,6 @@ class ListShortUrlsTest extends ApiTestCase 'shortUrl' => 'http://some-domain.com/custom-with-domain', 'longUrl' => 'https://google.com', 'dateCreated' => '2018-10-20T00:00:00+00:00', - 'visitsCount' => 0, 'visitsSummary' => [ 'total' => 0, 'nonBots' => 0, @@ -88,7 +85,6 @@ class ListShortUrlsTest extends ApiTestCase 'https://blog.alejandrocelaya.com/2017/12/09' . '/acmailer-7-0-the-most-important-release-in-a-long-time/', 'dateCreated' => '2019-01-01T00:00:10+00:00', - 'visitsCount' => 2, 'visitsSummary' => [ 'total' => 2, 'nonBots' => 1, @@ -110,7 +106,6 @@ class ListShortUrlsTest extends ApiTestCase 'shortUrl' => 'http://s.test/custom', 'longUrl' => 'https://shlink.io', 'dateCreated' => '2019-01-01T00:00:20+00:00', - 'visitsCount' => 0, 'visitsSummary' => [ 'total' => 0, 'nonBots' => 0, @@ -134,7 +129,6 @@ class ListShortUrlsTest extends ApiTestCase 'https://blog.alejandrocelaya.com/2019/04/27' . '/considerations-to-properly-use-open-source-software-projects/', 'dateCreated' => '2019-01-01T00:00:30+00:00', - 'visitsCount' => 0, 'visitsSummary' => [ 'total' => 0, 'nonBots' => 0, @@ -310,7 +304,7 @@ class ListShortUrlsTest extends ApiTestCase self::assertEquals([ 'invalidElements' => $expectedInvalidElements, 'title' => 'Invalid data', - 'type' => 'INVALID_ARGUMENT', + 'type' => 'https://shlink.io/api/error/invalid-data', 'status' => 400, 'detail' => 'Provided data is not valid', ], $respPayload); diff --git a/module/Rest/test-api/Action/RenameTagTest.php b/module/Rest/test-api/Action/RenameTagTest.php index e401da1d..35a3e1b2 100644 --- a/module/Rest/test-api/Action/RenameTagTest.php +++ b/module/Rest/test-api/Action/RenameTagTest.php @@ -24,7 +24,7 @@ class RenameTagTest extends ApiTestCase self::assertEquals(self::STATUS_FORBIDDEN, $resp->getStatusCode()); self::assertEquals(self::STATUS_FORBIDDEN, $payload['status']); - self::assertEquals('FORBIDDEN_OPERATION', $payload['type']); + self::assertEquals('https://shlink.io/api/error/forbidden-tag-operation', $payload['type']); self::assertEquals('You are not allowed to rename tags', $payload['detail']); self::assertEquals('Forbidden tag operation', $payload['title']); } diff --git a/module/Rest/test-api/Action/ResolveShortUrlTest.php b/module/Rest/test-api/Action/ResolveShortUrlTest.php index c10abc74..0c0ce5ec 100644 --- a/module/Rest/test-api/Action/ResolveShortUrlTest.php +++ b/module/Rest/test-api/Action/ResolveShortUrlTest.php @@ -58,7 +58,7 @@ class ResolveShortUrlTest extends ApiTestCase self::assertEquals(self::STATUS_NOT_FOUND, $resp->getStatusCode()); self::assertEquals(self::STATUS_NOT_FOUND, $payload['status']); - self::assertEquals('INVALID_SHORTCODE', $payload['type']); + self::assertEquals('https://shlink.io/api/error/short-url-not-found', $payload['type']); self::assertEquals($expectedDetail, $payload['detail']); self::assertEquals('Short URL not found', $payload['title']); self::assertEquals($shortCode, $payload['shortCode']); diff --git a/module/Rest/test-api/Action/ShortUrlVisitsTest.php b/module/Rest/test-api/Action/ShortUrlVisitsTest.php index 6a7e6a7e..8db002c4 100644 --- a/module/Rest/test-api/Action/ShortUrlVisitsTest.php +++ b/module/Rest/test-api/Action/ShortUrlVisitsTest.php @@ -34,7 +34,7 @@ class ShortUrlVisitsTest extends ApiTestCase self::assertEquals(self::STATUS_NOT_FOUND, $resp->getStatusCode()); self::assertEquals(self::STATUS_NOT_FOUND, $payload['status']); - self::assertEquals('INVALID_SHORTCODE', $payload['type']); + self::assertEquals('https://shlink.io/api/error/short-url-not-found', $payload['type']); self::assertEquals($expectedDetail, $payload['detail']); self::assertEquals('Short URL not found', $payload['title']); self::assertEquals($shortCode, $payload['shortCode']); diff --git a/module/Rest/test-api/Action/SingleStepCreateShortUrlTest.php b/module/Rest/test-api/Action/SingleStepCreateShortUrlTest.php index faed281d..038e3f38 100644 --- a/module/Rest/test-api/Action/SingleStepCreateShortUrlTest.php +++ b/module/Rest/test-api/Action/SingleStepCreateShortUrlTest.php @@ -38,7 +38,7 @@ class SingleStepCreateShortUrlTest extends ApiTestCase self::assertEquals(self::STATUS_UNAUTHORIZED, $resp->getStatusCode()); self::assertEquals(self::STATUS_UNAUTHORIZED, $payload['status']); - self::assertEquals('INVALID_AUTHORIZATION', $payload['type']); + self::assertEquals('https://shlink.io/api/error/missing-authentication', $payload['type']); self::assertEquals($expectedDetail, $payload['detail']); self::assertEquals('Invalid authorization', $payload['title']); } diff --git a/module/Rest/test-api/Action/TagVisitsTest.php b/module/Rest/test-api/Action/TagVisitsTest.php index fc54c111..c51f02fb 100644 --- a/module/Rest/test-api/Action/TagVisitsTest.php +++ b/module/Rest/test-api/Action/TagVisitsTest.php @@ -53,7 +53,7 @@ class TagVisitsTest extends ApiTestCase self::assertEquals(self::STATUS_NOT_FOUND, $resp->getStatusCode()); self::assertEquals(self::STATUS_NOT_FOUND, $payload['status']); - self::assertEquals('TAG_NOT_FOUND', $payload['type']); + self::assertEquals('https://shlink.io/api/error/tag-not-found', $payload['type']); self::assertEquals(sprintf('Tag with name "%s" could not be found', $tag), $payload['detail']); self::assertEquals('Tag not found', $payload['title']); } diff --git a/module/Rest/test-api/Action/TagsStatsTest.php b/module/Rest/test-api/Action/TagsStatsTest.php index 4ee94d42..9bf01474 100644 --- a/module/Rest/test-api/Action/TagsStatsTest.php +++ b/module/Rest/test-api/Action/TagsStatsTest.php @@ -25,29 +25,12 @@ class TagsStatsTest extends ApiTestCase self::assertEquals($expectedPagination, $tags['pagination']); } - #[Test, DataProvider('provideQueries')] - public function expectedListOfTagsIsReturnedForDeprecatedApproach( - string $apiKey, - array $query, - array $expectedStats, - array $expectedPagination, - ): void { - $query['withStats'] = 'true'; - $resp = $this->callApiWithKey(self::METHOD_GET, '/tags', [RequestOptions::QUERY => $query], $apiKey); - ['tags' => $tags] = $this->getJsonResponsePayload($resp); - - self::assertEquals($expectedStats, $tags['stats']); - self::assertEquals($expectedPagination, $tags['pagination']); - self::assertArrayHasKey('data', $tags); - } - public static function provideQueries(): iterable { yield 'admin API key' => ['valid_api_key', [], [ [ 'tag' => 'bar', 'shortUrlsCount' => 1, - 'visitsCount' => 2, 'visitsSummary' => [ 'total' => 2, 'nonBots' => 1, @@ -57,7 +40,6 @@ class TagsStatsTest extends ApiTestCase [ 'tag' => 'baz', 'shortUrlsCount' => 0, - 'visitsCount' => 0, 'visitsSummary' => [ 'total' => 0, 'nonBots' => 0, @@ -67,7 +49,6 @@ class TagsStatsTest extends ApiTestCase [ 'tag' => 'foo', 'shortUrlsCount' => 3, - 'visitsCount' => 5, 'visitsSummary' => [ 'total' => 5, 'nonBots' => 4, @@ -85,7 +66,6 @@ class TagsStatsTest extends ApiTestCase [ 'tag' => 'bar', 'shortUrlsCount' => 1, - 'visitsCount' => 2, 'visitsSummary' => [ 'total' => 2, 'nonBots' => 1, @@ -95,7 +75,6 @@ class TagsStatsTest extends ApiTestCase [ 'tag' => 'baz', 'shortUrlsCount' => 0, - 'visitsCount' => 0, 'visitsSummary' => [ 'total' => 0, 'nonBots' => 0, @@ -113,7 +92,6 @@ class TagsStatsTest extends ApiTestCase [ 'tag' => 'bar', 'shortUrlsCount' => 1, - 'visitsCount' => 2, 'visitsSummary' => [ 'total' => 2, 'nonBots' => 1, @@ -123,7 +101,6 @@ class TagsStatsTest extends ApiTestCase [ 'tag' => 'foo', 'shortUrlsCount' => 2, - 'visitsCount' => 5, 'visitsSummary' => [ 'total' => 5, 'nonBots' => 4, @@ -141,7 +118,6 @@ class TagsStatsTest extends ApiTestCase [ 'tag' => 'foo', 'shortUrlsCount' => 2, - 'visitsCount' => 5, 'visitsSummary' => [ 'total' => 5, 'nonBots' => 4, @@ -159,7 +135,6 @@ class TagsStatsTest extends ApiTestCase [ 'tag' => 'foo', 'shortUrlsCount' => 1, - 'visitsCount' => 0, 'visitsSummary' => [ 'total' => 0, 'nonBots' => 0, diff --git a/module/Rest/test-api/Action/UpdateTagTest.php b/module/Rest/test-api/Action/UpdateTagTest.php index 96b8ed62..3bced135 100644 --- a/module/Rest/test-api/Action/UpdateTagTest.php +++ b/module/Rest/test-api/Action/UpdateTagTest.php @@ -23,7 +23,7 @@ class UpdateTagTest extends ApiTestCase self::assertEquals(self::STATUS_BAD_REQUEST, $resp->getStatusCode()); self::assertEquals(self::STATUS_BAD_REQUEST, $payload['status']); - self::assertEquals('INVALID_ARGUMENT', $payload['type']); + self::assertEquals('https://shlink.io/api/error/invalid-data', $payload['type']); self::assertEquals($expectedDetail, $payload['detail']); self::assertEquals('Invalid data', $payload['title']); } @@ -55,8 +55,8 @@ class UpdateTagTest extends ApiTestCase public static function provideTagNotFoundApiVersions(): iterable { - yield 'version 1' => ['1', 'TAG_NOT_FOUND']; - yield 'version 2' => ['2', 'TAG_NOT_FOUND']; + yield 'version 1' => ['1', 'https://shlink.io/api/error/tag-not-found']; + yield 'version 2' => ['2', 'https://shlink.io/api/error/tag-not-found']; yield 'version 3' => ['3', 'https://shlink.io/api/error/tag-not-found']; } @@ -80,8 +80,8 @@ class UpdateTagTest extends ApiTestCase public static function provideTagConflictsApiVersions(): iterable { - yield 'version 1' => ['1', 'TAG_CONFLICT']; - yield 'version 2' => ['2', 'TAG_CONFLICT']; + yield 'version 1' => ['1', 'https://shlink.io/api/error/tag-conflict']; + yield 'version 2' => ['2', 'https://shlink.io/api/error/tag-conflict']; yield 'version 3' => ['3', 'https://shlink.io/api/error/tag-conflict']; } diff --git a/module/Rest/test-api/Action/VisitStatsTest.php b/module/Rest/test-api/Action/VisitStatsTest.php index 10a4de0c..2adf5a6a 100644 --- a/module/Rest/test-api/Action/VisitStatsTest.php +++ b/module/Rest/test-api/Action/VisitStatsTest.php @@ -32,8 +32,6 @@ class VisitStatsTest extends ApiTestCase 'nonBots' => 2, 'bots' => 1, ], - 'visitsCount' => 7, - 'orphanVisitsCount' => 3, ]]; yield 'domain-only API key' => ['domain_api_key', [ 'nonOrphanVisits' => [ @@ -46,8 +44,6 @@ class VisitStatsTest extends ApiTestCase 'nonBots' => 2, 'bots' => 1, ], - 'visitsCount' => 0, - 'orphanVisitsCount' => 3, ]]; yield 'author API key' => ['author_api_key', [ 'nonOrphanVisits' => [ @@ -60,8 +56,6 @@ class VisitStatsTest extends ApiTestCase 'nonBots' => 2, 'bots' => 1, ], - 'visitsCount' => 5, - 'orphanVisitsCount' => 3, ]]; } } diff --git a/module/Rest/test-api/Middleware/AuthenticationTest.php b/module/Rest/test-api/Middleware/AuthenticationTest.php index d086f6a6..1c164c85 100644 --- a/module/Rest/test-api/Middleware/AuthenticationTest.php +++ b/module/Rest/test-api/Middleware/AuthenticationTest.php @@ -29,8 +29,8 @@ class AuthenticationTest extends ApiTestCase public static function provideApiVersions(): iterable { - yield 'version 1' => ['1', 'INVALID_AUTHORIZATION']; - yield 'version 2' => ['2', 'INVALID_AUTHORIZATION']; + yield 'version 1' => ['1', 'https://shlink.io/api/error/missing-authentication']; + yield 'version 2' => ['2', 'https://shlink.io/api/error/missing-authentication']; yield 'version 3' => ['3', 'https://shlink.io/api/error/missing-authentication']; } @@ -58,9 +58,9 @@ class AuthenticationTest extends ApiTestCase public static function provideInvalidApiKeys(): iterable { - yield 'key which does not exist' => ['invalid', '2', 'INVALID_API_KEY']; - yield 'key which is expired' => ['expired_api_key', '2', 'INVALID_API_KEY']; - yield 'key which is disabled' => ['disabled_api_key', '2', 'INVALID_API_KEY']; + yield 'key which does not exist' => ['invalid', '2', 'https://shlink.io/api/error/invalid-api-key']; + yield 'key which is expired' => ['expired_api_key', '2', 'https://shlink.io/api/error/invalid-api-key']; + yield 'key which is disabled' => ['disabled_api_key', '2', 'https://shlink.io/api/error/invalid-api-key']; yield 'version 3' => ['disabled_api_key', '3', 'https://shlink.io/api/error/invalid-api-key']; } } diff --git a/module/Rest/test/Action/Tag/ListTagsActionTest.php b/module/Rest/test/Action/Tag/ListTagsActionTest.php index 447e8331..a63041dd 100644 --- a/module/Rest/test/Action/Tag/ListTagsActionTest.php +++ b/module/Rest/test/Action/Tag/ListTagsActionTest.php @@ -7,14 +7,12 @@ namespace ShlinkioTest\Shlink\Rest\Action\Tag; use Laminas\Diactoros\Response\JsonResponse; use Laminas\Diactoros\ServerRequestFactory; use Pagerfanta\Adapter\ArrayAdapter; -use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; use Psr\Http\Message\ServerRequestInterface; use Shlinkio\Shlink\Common\Paginator\Paginator; use Shlinkio\Shlink\Core\Tag\Entity\Tag; -use Shlinkio\Shlink\Core\Tag\Model\TagInfo; use Shlinkio\Shlink\Core\Tag\TagServiceInterface; use Shlinkio\Shlink\Rest\Action\Tag\ListTagsAction; use Shlinkio\Shlink\Rest\Entity\ApiKey; @@ -32,8 +30,8 @@ class ListTagsActionTest extends TestCase $this->action = new ListTagsAction($this->tagService); } - #[Test, DataProvider('provideNoStatsQueries')] - public function returnsBaseDataWhenStatsAreNotRequested(array $query): void + #[Test] + public function returnsBaseDataWhenStatsAreNotRequested(): void { $tags = [new Tag('foo'), new Tag('bar')]; $tagsCount = count($tags); @@ -43,7 +41,7 @@ class ListTagsActionTest extends TestCase )->willReturn(new Paginator(new ArrayAdapter($tags))); /** @var JsonResponse $resp */ - $resp = $this->action->handle($this->requestWithApiKey()->withQueryParams($query)); + $resp = $this->action->handle($this->requestWithApiKey()); $payload = $resp->getPayload(); self::assertEquals([ @@ -60,46 +58,6 @@ class ListTagsActionTest extends TestCase ], $payload); } - public static function provideNoStatsQueries(): iterable - { - yield 'no query' => [[]]; - yield 'withStats is false' => [['withStats' => 'withStats']]; - yield 'withStats is something else' => [['withStats' => 'foo']]; - } - - #[Test] - public function returnsStatsWhenRequested(): void - { - $stats = [ - new TagInfo('foo', 1, 1), - new TagInfo('bar', 3, 10), - ]; - $itemsCount = count($stats); - $this->tagService->expects($this->once())->method('tagsInfo')->with( - $this->anything(), - $this->isInstanceOf(ApiKey::class), - )->willReturn(new Paginator(new ArrayAdapter($stats))); - $req = $this->requestWithApiKey()->withQueryParams(['withStats' => 'true']); - - /** @var JsonResponse $resp */ - $resp = $this->action->handle($req); - $payload = $resp->getPayload(); - - self::assertEquals([ - 'tags' => [ - 'data' => ['foo', 'bar'], - 'stats' => $stats, - 'pagination' => [ - 'currentPage' => 1, - 'pagesCount' => 1, - 'itemsPerPage' => 10, - 'itemsInCurrentPage' => $itemsCount, - 'totalItems' => $itemsCount, - ], - ], - ], $payload); - } - private function requestWithApiKey(): ServerRequestInterface { return ServerRequestFactory::fromGlobals()->withAttribute(ApiKey::class, ApiKey::create()); diff --git a/module/Rest/test/Exception/BackwardsCompatibleProblemDetailsExceptionTest.php b/module/Rest/test/Exception/BackwardsCompatibleProblemDetailsExceptionTest.php deleted file mode 100644 index e51a5ac1..00000000 --- a/module/Rest/test/Exception/BackwardsCompatibleProblemDetailsExceptionTest.php +++ /dev/null @@ -1,113 +0,0 @@ -<?php - -declare(strict_types=1); - -namespace ShlinkioTest\Shlink\Rest\Exception; - -use Exception; -use Mezzio\ProblemDetails\Exception\ProblemDetailsExceptionInterface; -use PHPUnit\Framework\Attributes\DataProvider; -use PHPUnit\Framework\Attributes\Test; -use PHPUnit\Framework\TestCase; -use Shlinkio\Shlink\Core\Exception\DeleteShortUrlException; -use Shlinkio\Shlink\Core\Exception\DomainNotFoundException; -use Shlinkio\Shlink\Core\Exception\ForbiddenTagOperationException; -use Shlinkio\Shlink\Core\Exception\InvalidUrlException; -use Shlinkio\Shlink\Core\Exception\NonUniqueSlugException; -use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; -use Shlinkio\Shlink\Core\Exception\TagConflictException; -use Shlinkio\Shlink\Core\Exception\TagNotFoundException; -use Shlinkio\Shlink\Core\Exception\ValidationException; -use Shlinkio\Shlink\Rest\Exception\BackwardsCompatibleProblemDetailsException; -use Shlinkio\Shlink\Rest\Exception\MercureException; -use Shlinkio\Shlink\Rest\Exception\MissingAuthenticationException; -use Shlinkio\Shlink\Rest\Exception\VerifyAuthenticationException; - -class BackwardsCompatibleProblemDetailsExceptionTest extends TestCase -{ - #[Test, DataProvider('provideTypes')] - public function typeIsRemappedOnWrappedException( - string $wrappedType, - string $expectedType, - bool $expectSameType = false, - ): void { - $original = new class ($wrappedType) extends Exception implements ProblemDetailsExceptionInterface { - public function __construct(private readonly string $type) - { - parent::__construct(''); - } - - public function getStatus(): int - { - return 123; - } - - public function getType(): string - { - return $this->type; - } - - public function getTitle(): string - { - return 'title'; - } - - public function getDetail(): string - { - return 'detail'; - } - - public function getAdditionalData(): array - { - return []; - } - - public function toArray(): array - { - return ['type' => $this->type]; - } - - public function jsonSerialize(): array - { - return ['type' => $this->type]; - } - }; - $e = BackwardsCompatibleProblemDetailsException::fromProblemDetails($original); - - self::assertEquals($e->getType(), $expectedType); - self::assertEquals($e->toArray(), ['type' => $expectedType]); - self::assertEquals($e->jsonSerialize(), ['type' => $expectedType]); - - self::assertEquals($original->getTitle(), $e->getTitle()); - self::assertEquals($original->getDetail(), $e->getDetail()); - self::assertEquals($original->getAdditionalData(), $e->getAdditionalData()); - - if ($expectSameType) { - self::assertEquals($original->getType(), $e->getType()); - self::assertEquals($original->toArray(), $e->toArray()); - self::assertEquals($original->jsonSerialize(), $e->jsonSerialize()); - } else { - self::assertNotEquals($original->getType(), $e->getType()); - self::assertNotEquals($original->toArray(), $e->toArray()); - self::assertNotEquals($original->jsonSerialize(), $e->jsonSerialize()); - } - } - - public static function provideTypes(): iterable - { - yield ['foo', 'foo', true]; - yield ['bar', 'bar', true]; - yield [ValidationException::ERROR_CODE, 'INVALID_ARGUMENT']; - yield [DeleteShortUrlException::ERROR_CODE, 'INVALID_SHORT_URL_DELETION']; - yield [DomainNotFoundException::ERROR_CODE, 'DOMAIN_NOT_FOUND']; - yield [ForbiddenTagOperationException::ERROR_CODE, 'FORBIDDEN_OPERATION']; - yield [InvalidUrlException::ERROR_CODE, 'INVALID_URL']; - yield [NonUniqueSlugException::ERROR_CODE, 'INVALID_SLUG']; - yield [ShortUrlNotFoundException::ERROR_CODE, 'INVALID_SHORTCODE']; - yield [TagConflictException::ERROR_CODE, 'TAG_CONFLICT']; - yield [TagNotFoundException::ERROR_CODE, 'TAG_NOT_FOUND']; - yield [MercureException::ERROR_CODE, 'MERCURE_NOT_CONFIGURED']; - yield [MissingAuthenticationException::ERROR_CODE, 'INVALID_AUTHORIZATION']; - yield [VerifyAuthenticationException::ERROR_CODE, 'INVALID_API_KEY']; - } -} diff --git a/module/Rest/test/Middleware/ErrorHandler/BackwardsCompatibleProblemDetailsHandlerTest.php b/module/Rest/test/Middleware/ErrorHandler/BackwardsCompatibleProblemDetailsHandlerTest.php deleted file mode 100644 index 78862980..00000000 --- a/module/Rest/test/Middleware/ErrorHandler/BackwardsCompatibleProblemDetailsHandlerTest.php +++ /dev/null @@ -1,74 +0,0 @@ -<?php - -declare(strict_types=1); - -namespace ShlinkioTest\Shlink\Rest\Middleware\ErrorHandler; - -use Laminas\Diactoros\ServerRequestFactory; -use PHPUnit\Framework\Attributes\DataProvider; -use PHPUnit\Framework\Attributes\Test; -use PHPUnit\Framework\TestCase; -use Psr\Http\Message\ServerRequestInterface; -use Psr\Http\Server\RequestHandlerInterface; -use Shlinkio\Shlink\Core\Exception\ValidationException; -use Shlinkio\Shlink\Rest\Exception\BackwardsCompatibleProblemDetailsException; -use Shlinkio\Shlink\Rest\Middleware\ErrorHandler\BackwardsCompatibleProblemDetailsHandler; -use Throwable; - -class BackwardsCompatibleProblemDetailsHandlerTest extends TestCase -{ - private BackwardsCompatibleProblemDetailsHandler $handler; - - protected function setUp(): void - { - $this->handler = new BackwardsCompatibleProblemDetailsHandler(); - } - - /** - * @param class-string<Throwable> $expectedException - */ - #[Test, DataProvider('provideExceptions')] - public function expectedExceptionIsThrownBasedOnTheRequestVersion( - ServerRequestInterface $request, - Throwable $thrownException, - string $expectedException, - ): void { - $handler = $this->createMock(RequestHandlerInterface::class); - $handler->expects($this->once())->method('handle')->with($request)->willThrowException($thrownException); - - $this->expectException($expectedException); - - $this->handler->process($request, $handler); - } - - public static function provideExceptions(): iterable - { - $baseRequest = ServerRequestFactory::fromGlobals(); - - yield 'no version' => [ - $baseRequest, - ValidationException::fromArray([]), - BackwardsCompatibleProblemDetailsException::class, - ]; - yield 'version 1' => [ - $baseRequest->withAttribute('version', '1'), - ValidationException::fromArray([]), - BackwardsCompatibleProblemDetailsException::class, - ]; - yield 'version 2' => [ - $baseRequest->withAttribute('version', '2'), - ValidationException::fromArray([]), - BackwardsCompatibleProblemDetailsException::class, - ]; - yield 'version 3' => [ - $baseRequest->withAttribute('version', '3'), - ValidationException::fromArray([]), - ValidationException::class, - ]; - yield 'version 4' => [ - $baseRequest->withAttribute('version', '3'), - ValidationException::fromArray([]), - ValidationException::class, - ]; - } -}