diff --git a/.travis.yml b/.travis.yml index e4d43204..ce89e4bb 100644 --- a/.travis.yml +++ b/.travis.yml @@ -20,7 +20,7 @@ before_install: - sudo ./data/infra/ci/install-docker.sh - sudo ./data/infra/ci/install-ms-odbc.sh - docker-compose -f docker-compose.yml -f docker-compose.ci.yml up -d shlink_db_ms shlink_db shlink_db_postgres shlink_db_maria - - yes | pecl install pdo_sqlsrv swoole-4.4.15 + - yes | pecl install pdo_sqlsrv swoole-4.4.18 - echo 'extension = apcu.so' >> ~/.phpenv/versions/$(phpenv version-name)/etc/php.ini - phpenv config-rm xdebug.ini || return 0 diff --git a/CHANGELOG.md b/CHANGELOG.md index 67105b4d..4ede9184 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,30 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com), and this project adheres to [Semantic Versioning](https://semver.org). -## [Unreleased] +## 2.2.1 - 2020-05-11 + +#### Added + +* *Nothing* + +#### Changed + +* *Nothing* + +#### Deprecated + +* *Nothing* + +#### Removed + +* *Nothing* + +#### Fixed + +* [#764](https://github.com/shlinkio/shlink/issues/764) Fixed error when trying to match an existing short URL which does not have `validSince` and/or `validUntil`, but you are providing either one of them for the new one. + + +## 2.2.0 - 2020-05-09 #### Added @@ -19,14 +42,26 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this The updates are only published when serving Shlink with swoole. - Also, Shlink exposes a new endpoint `GET /rest/v2/mercure-info`, which returns the public URL of the mercure hub, and a valid JWT that can be used to subsribe to updates. + Also, Shlink exposes a new endpoint `GET /rest/v2/mercure-info`, which returns the public URL of the mercure hub, and a valid JWT that can be used to subscribe to updates. * [#673](https://github.com/shlinkio/shlink/issues/673) Added new `[GET /visits]` rest endpoint which returns basic visits stats. +* [#674](https://github.com/shlinkio/shlink/issues/674) Added new `[GET /tags/{tag}/visits]` rest endpoint which returns visits by tag. + + It works in the same way as the `[GET /short-urls/{shortCode}/visits]` one, returning the same response payload, and supporting the same query params, but the response is the list of visits in all short URLs which have provided tag. + +* [#672](https://github.com/shlinkio/shlink/issues/672) Enhanced `[GET /tags]` rest endpoint so that it is possible to get basic stats info for every tag. + + Now, if the `withStats=true` query param is provided, the response payload will include a new `stats` property which is a list with the amount of short URLs and visits for every tag. + + Also, the `tag:list` CLI command has been changed and it always behaves like this. + +* [#640](https://github.com/shlinkio/shlink/issues/640) Allowed to optionally disable visitors' IP address anonymization. This will make Shlink no longer be GDPR-compliant, but it's OK if you only plan to share your URLs in countries without this regulation. #### Changed * [#692](https://github.com/shlinkio/shlink/issues/692) Drastically improved performance when loading visits. Specially noticeable when loading big result sets. * [#657](https://github.com/shlinkio/shlink/issues/657) Updated how DB tests are run in travis by using docker containers which allow all engines to be covered. +* [#751](https://github.com/shlinkio/shlink/issues/751) Updated PHP and swoole versions used in docker image, and removed mssql-tools, as they are not needed. #### Deprecated diff --git a/Dockerfile b/Dockerfile index edd8314a..a1b1f6f1 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,8 +1,8 @@ -FROM php:7.4.2-alpine3.11 as base +FROM php:7.4.5-alpine3.11 as base -ARG SHLINK_VERSION=2.0.5 +ARG SHLINK_VERSION=2.1.4 ENV SHLINK_VERSION ${SHLINK_VERSION} -ENV SWOOLE_VERSION 4.4.15 +ENV SWOOLE_VERSION 4.4.18 ENV LC_ALL "C" WORKDIR /etc/shlink @@ -26,15 +26,12 @@ RUN \ # Install sqlsrv driver RUN if [ $(uname -m) == "x86_64" ]; then \ wget https://download.microsoft.com/download/e/4/e/e4e67866-dffd-428c-aac7-8d28ddafb39b/msodbcsql17_17.5.1.1-1_amd64.apk && \ - wget https://download.microsoft.com/download/e/4/e/e4e67866-dffd-428c-aac7-8d28ddafb39b/mssql-tools_17.5.1.1-1_amd64.apk && \ apk add --allow-untrusted msodbcsql17_17.5.1.1-1_amd64.apk && \ - apk add --allow-untrusted mssql-tools_17.5.1.1-1_amd64.apk && \ apk add --no-cache --virtual .phpize-deps $PHPIZE_DEPS unixodbc-dev && \ pecl install pdo_sqlsrv && \ docker-php-ext-enable pdo_sqlsrv && \ apk del .phpize-deps && \ - rm msodbcsql17_17.5.1.1-1_amd64.apk && \ - rm mssql-tools_17.5.1.1-1_amd64.apk ; \ + rm msodbcsql17_17.5.1.1-1_amd64.apk ; \ fi # Install swoole diff --git a/composer.json b/composer.json index 5ba58a8e..5d531a37 100644 --- a/composer.json +++ b/composer.json @@ -48,10 +48,10 @@ "predis/predis": "^1.1", "pugx/shortid-php": "^0.5", "ramsey/uuid": "^3.9", - "shlinkio/shlink-common": "dev-master#e659cf9d9b5b3b131419e2f55f2e595f562baafc as 3.1.0", + "shlinkio/shlink-common": "^3.1.0", "shlinkio/shlink-config": "^1.0", "shlinkio/shlink-event-dispatcher": "^1.4", - "shlinkio/shlink-installer": "dev-master#dae6644587d0c1c59ca773722531551b9f436786 as 5.0.0", + "shlinkio/shlink-installer": "^5.0.0", "shlinkio/shlink-ip-geolocation": "^1.4", "symfony/console": "^5.0", "symfony/filesystem": "^5.0", diff --git a/config/autoload/installer.global.php b/config/autoload/installer.global.php index d46aa8d7..db1914db 100644 --- a/config/autoload/installer.global.php +++ b/config/autoload/installer.global.php @@ -36,6 +36,7 @@ return [ Option\Mercure\MercureInternalUrlConfigOption::class, Option\Mercure\MercureJwtSecretConfigOption::class, Option\UrlShortener\GeoLiteLicenseKeyConfigOption::class, + Option\UrlShortener\IpAnonymizationConfigOption::class, ], 'installation_commands' => [ diff --git a/config/autoload/url-shortener.global.php b/config/autoload/url-shortener.global.php index 165e0258..5ad66bea 100644 --- a/config/autoload/url-shortener.global.php +++ b/config/autoload/url-shortener.global.php @@ -12,6 +12,7 @@ return [ 'hostname' => '', ], 'validate_url' => false, + 'anonymize_remote_addr' => true, 'visits_webhooks' => [], 'default_short_codes_length' => DEFAULT_SHORT_CODES_LENGTH, ], diff --git a/data/infra/php.Dockerfile b/data/infra/php.Dockerfile index c5401651..33b654c5 100644 --- a/data/infra/php.Dockerfile +++ b/data/infra/php.Dockerfile @@ -1,4 +1,4 @@ -FROM php:7.4.2-fpm-alpine3.11 +FROM php:7.4.5-fpm-alpine3.11 MAINTAINER Alejandro Celaya ENV APCU_VERSION 5.1.18 @@ -67,15 +67,12 @@ RUN rm /tmp/xdebug.tar.gz # Install sqlsrv driver RUN wget https://download.microsoft.com/download/e/4/e/e4e67866-dffd-428c-aac7-8d28ddafb39b/msodbcsql17_17.5.1.1-1_amd64.apk && \ - wget https://download.microsoft.com/download/e/4/e/e4e67866-dffd-428c-aac7-8d28ddafb39b/mssql-tools_17.5.1.1-1_amd64.apk && \ apk add --allow-untrusted msodbcsql17_17.5.1.1-1_amd64.apk && \ - apk add --allow-untrusted mssql-tools_17.5.1.1-1_amd64.apk && \ apk add --no-cache --virtual .phpize-deps $PHPIZE_DEPS unixodbc-dev && \ pecl install pdo_sqlsrv && \ docker-php-ext-enable pdo_sqlsrv && \ apk del .phpize-deps && \ - rm msodbcsql17_17.5.1.1-1_amd64.apk && \ - rm mssql-tools_17.5.1.1-1_amd64.apk + rm msodbcsql17_17.5.1.1-1_amd64.apk # Install composer RUN php -r "readfile('https://getcomposer.org/installer');" | php diff --git a/data/infra/swoole.Dockerfile b/data/infra/swoole.Dockerfile index 3f7a1513..9d8d4240 100644 --- a/data/infra/swoole.Dockerfile +++ b/data/infra/swoole.Dockerfile @@ -1,10 +1,10 @@ -FROM php:7.4.2-alpine3.11 +FROM php:7.4.5-alpine3.11 MAINTAINER Alejandro Celaya ENV APCU_VERSION 5.1.18 ENV APCU_BC_VERSION 1.0.5 ENV INOTIFY_VERSION 2.0.0 -ENV SWOOLE_VERSION 4.4.15 +ENV SWOOLE_VERSION 4.4.18 RUN apk update @@ -68,15 +68,12 @@ RUN rm /tmp/inotify.tar.gz # Install swoole and mssql driver RUN wget https://download.microsoft.com/download/e/4/e/e4e67866-dffd-428c-aac7-8d28ddafb39b/msodbcsql17_17.5.1.1-1_amd64.apk && \ - wget https://download.microsoft.com/download/e/4/e/e4e67866-dffd-428c-aac7-8d28ddafb39b/mssql-tools_17.5.1.1-1_amd64.apk && \ apk add --allow-untrusted msodbcsql17_17.5.1.1-1_amd64.apk && \ - apk add --allow-untrusted mssql-tools_17.5.1.1-1_amd64.apk && \ apk add --no-cache --virtual .phpize-deps $PHPIZE_DEPS unixodbc-dev && \ pecl install swoole-${SWOOLE_VERSION} pdo_sqlsrv && \ docker-php-ext-enable swoole pdo_sqlsrv && \ apk del .phpize-deps && \ - rm msodbcsql17_17.5.1.1-1_amd64.apk && \ - rm mssql-tools_17.5.1.1-1_amd64.apk + rm msodbcsql17_17.5.1.1-1_amd64.apk # Install composer RUN php -r "readfile('https://getcomposer.org/installer');" | php diff --git a/docker-compose.yml b/docker-compose.yml index 36153ad2..d700f3b3 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -79,7 +79,7 @@ services: shlink_db_postgres: container_name: shlink_db_postgres - image: postgres:10.7-alpine + image: postgres:12.2-alpine ports: - "5433:5432" volumes: @@ -92,7 +92,7 @@ services: shlink_db_maria: container_name: shlink_db_maria - image: mariadb:10.2 + image: mariadb:10.5 ports: - "3308:3306" volumes: @@ -114,7 +114,7 @@ services: shlink_redis: container_name: shlink_redis - image: redis:5.0-alpine + image: redis:6.0-alpine ports: - "6380:6379" @@ -131,7 +131,7 @@ services: shlink_mercure: container_name: shlink_mercure - image: dunglas/mercure:v0.8 + image: dunglas/mercure:v0.9 ports: - "3080:80" environment: diff --git a/docker/README.md b/docker/README.md index aa9ae16b..e17e570e 100644 --- a/docker/README.md +++ b/docker/README.md @@ -168,12 +168,12 @@ This is the complete list of supported env vars: * `TASK_WORKER_NUM`: The amount of concurrent background tasks this shlink instance will be able to execute. Defaults to 16. * `VISITS_WEBHOOKS`: A comma-separated list of URLs that will receive a `POST` request when a short URL receives a visit. * `DEFAULT_SHORT_CODES_LENGTH`: The length you want generated short codes to have. It defaults to 5 and has to be at least 4, so any value smaller than that will fall back to 4. +* `GEOLITE_LICENSE_KEY`: The license key used to download new GeoLite2 database files. This is not mandatory, as a default license key is provided, but it is **strongly recommended** that you provide your own. Go to [https://shlink.io/documentation/geolite-license-key](https://shlink.io/documentation/geolite-license-key) to know how to generate it. * `REDIS_SERVERS`: A comma-separated list of redis servers where Shlink locks are stored (locks are used to prevent some operations to be run more than once in parallel). * `MERCURE_PUBLIC_HUB_URL`: The public URL of a mercure hub server to which Shlink will sent updates. This URL will also be served to consumers that want to subscribe to those updates. * `MERCURE_INTERNAL_HUB_URL`: An internal URL for a mercure hub. Will be used only when publishing updates to mercure, and does not need to be public. If this is not provided but `MERCURE_PUBLIC_HUB_URL` was, the former one will be used to publish updates. * `MERCURE_JWT_SECRET`: The secret key that was provided to the mercure hub server, in order to be able to generate valid JWTs for publishing/subscribing to that server. - -* `GEOLITE_LICENSE_KEY`: The license key used to download new GeoLite2 database files. This is not mandatory, as a default license key is provided, but it is **strongly recommended** that you provide your own. Go to [https://shlink.io/documentation/geolite-license-key](https://shlink.io/documentation/geolite-license-key) to know how to generate it. +* `ANONYMIZE_REMOTE_ADDR`: Tells if IP addresses from visitors should be obfuscated before storing them in the database. Default value is `true`. **Careful!** Setting this to `false` will make your Shlink instance no longer be in compliance with the GDPR and other similar data protection regulations. An example using all env vars could look like this: @@ -205,6 +205,7 @@ docker run \ -e "MERCURE_PUBLIC_HUB_URL=https://example.com" \ -e "MERCURE_INTERNAL_HUB_URL=http://my-mercure-hub.prod.svc.cluster.local" \ -e MERCURE_JWT_SECRET=super_secret_key \ + -e ANONYMIZE_REMOTE_ADDR=false \ shlinkio/shlink:stable ``` @@ -249,7 +250,8 @@ The whole configuration should have this format, but it can be split into multip "geolite_license_key": "kjh23ljkbndskj345", "mercure_public_hub_url": "https://example.com", "mercure_internal_hub_url": "http://my-mercure-hub.prod.svc.cluster.local", - "mercure_jwt_secret": "super_secret_key" + "mercure_jwt_secret": "super_secret_key", + "anonymize_remote_addr": false } ``` diff --git a/docker/build b/docker/build index 34ee7a21..22580702 100755 --- a/docker/build +++ b/docker/build @@ -14,10 +14,15 @@ echo "$DOCKER_PASSWORD" | docker login -u "$DOCKER_USERNAME" --password-stdin # If there is a tag, regardless the branch, build that docker tag and also "stable" if [[ ! -z $TRAVIS_TAG ]]; then + TAGS="-t shlinkio/shlink:${TRAVIS_TAG#?}" + # Push stable tag only if this is not an alpha or beta tag + [[ $TRAVIS_TAG != *"alpha"* && $TRAVIS_TAG != *"beta"* ]] && TAGS="${TAGS} -t shlinkio/shlink:stable" + docker buildx build --push \ --build-arg SHLINK_VERSION=${TRAVIS_TAG#?} \ --platform linux/arm/v7,linux/arm64/v8,linux/amd64 \ - -t shlinkio/shlink:${TRAVIS_TAG#?} -t shlinkio/shlink:stable . + ${TAGS} . + # If build branch is develop, build latest (on master, when there's no tag, do not build anything) elif [[ "$TRAVIS_BRANCH" == 'develop' ]]; then docker buildx build --push \ diff --git a/docker/config/shlink_in_docker.local.php b/docker/config/shlink_in_docker.local.php index 5662aaee..b870ccd7 100644 --- a/docker/config/shlink_in_docker.local.php +++ b/docker/config/shlink_in_docker.local.php @@ -117,6 +117,7 @@ return [ 'hostname' => env('SHORT_DOMAIN_HOST', ''), ], 'validate_url' => (bool) env('VALIDATE_URLS', false), + 'anonymize_remote_addr' => (bool) env('ANONYMIZE_REMOTE_ADDR', true), 'visits_webhooks' => $helper->getVisitsWebhooks(), 'default_short_codes_length' => $helper->getDefaultShortCodesLength(), ], diff --git a/docs/swagger/definitions/TagInfo.json b/docs/swagger/definitions/TagInfo.json new file mode 100644 index 00000000..e881ce02 --- /dev/null +++ b/docs/swagger/definitions/TagInfo.json @@ -0,0 +1,17 @@ +{ + "type": "object", + "properties": { + "tag": { + "type": "string", + "description": "The unique tag name" + }, + "shortUrlsCount": { + "type": "number", + "description": "The amount of short URLs using this tag" + }, + "userAgent": { + "type": "number", + "description": "The combined amount of visits received by short URLs with this tag" + } + } +} diff --git a/docs/swagger/paths/v1_tags.json b/docs/swagger/paths/v1_tags.json index 5e7fd71c..83bc7d68 100644 --- a/docs/swagger/paths/v1_tags.json +++ b/docs/swagger/paths/v1_tags.json @@ -14,6 +14,19 @@ "parameters": [ { "$ref": "../parameters/version.json" + }, + { + "name": "withStats", + "description": "Whether you want to include also a list with general stats by tag or not.", + "in": "query", + "required": false, + "schema": { + "type": "string", + "enum": [ + "true", + "false" + ] + } } ], "responses": { @@ -26,12 +39,20 @@ "properties": { "tags": { "type": "object", + "required": ["data"], "properties": { "data": { "type": "array", "items": { "type": "string" } + }, + "stats": { + "description": "The tag stats will be returned only if the withStats param was provided with value 'true'", + "type": "array", + "items": { + "$ref": "../definitions/TagInfo.json" + } } } } diff --git a/docs/swagger/paths/v2_tags_{tag}_visits.json b/docs/swagger/paths/v2_tags_{tag}_visits.json new file mode 100644 index 00000000..d9d9dda7 --- /dev/null +++ b/docs/swagger/paths/v2_tags_{tag}_visits.json @@ -0,0 +1,154 @@ +{ + "get": { + "operationId": "getTagVisits", + "tags": [ + "Visits" + ], + "summary": "List visits for tag", + "description": "Get the list of visits on any short URL which is tagged with provided tag.", + "parameters": [ + { + "$ref": "../parameters/version.json" + }, + { + "name": "tag", + "in": "path", + "description": "The tag from which we want to get the visits.", + "required": true, + "schema": { + "type": "string" + } + }, + { + "name": "startDate", + "in": "query", + "description": "The date (in ISO-8601 format) from which we want to get visits.", + "required": false, + "schema": { + "type": "string" + } + }, + { + "name": "endDate", + "in": "query", + "description": "The date (in ISO-8601 format) until which we want to get visits.", + "required": false, + "schema": { + "type": "string" + } + }, + { + "name": "page", + "in": "query", + "description": "The page to display. Defaults to 1", + "required": false, + "schema": { + "type": "number" + } + }, + { + "name": "itemsPerPage", + "in": "query", + "description": "The amount of items to return on every page. Defaults to all the items", + "required": false, + "schema": { + "type": "number" + } + } + ], + "security": [ + { + "ApiKey": [] + } + ], + "responses": { + "200": { + "description": "List of visits.", + "content": { + "application/json": { + "schema": { + "type": "object", + "properties": { + "visits": { + "type": "object", + "properties": { + "data": { + "type": "array", + "items": { + "$ref": "../definitions/Visit.json" + } + }, + "pagination": { + "$ref": "../definitions/Pagination.json" + } + } + } + } + } + } + }, + "examples": { + "application/json": { + "visits": { + "data": [ + { + "referer": "https://twitter.com", + "date": "2015-08-20T05:05:03+04:00", + "userAgent": "Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:47.0) Gecko/20100101 Firefox/47.0 Mozilla/5.0 (Macintosh; Intel Mac OS X x.y; rv:42.0) Gecko/20100101 Firefox/42.0", + "visitLocation": null + }, + { + "referer": "https://t.co", + "date": "2015-08-20T05:05:03+04:00", + "userAgent": "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/51.0.2704.103 Safari/537.36", + "visitLocation": { + "cityName": "Cupertino", + "countryCode": "US", + "countryName": "United States", + "latitude": 37.3042, + "longitude": -122.0946, + "regionName": "California", + "timezone": "America/Los_Angeles" + } + }, + { + "referer": null, + "date": "2015-08-20T05:05:03+04:00", + "userAgent": "some_web_crawler/1.4", + "visitLocation": null + } + ], + "pagination": { + "currentPage": 5, + "pagesCount": 12, + "itemsPerPage": 10, + "itemsInCurrentPage": 10, + "totalItems": 115 + } + } + } + } + }, + "404": { + "description": "The tag does not exist.", + "content": { + "application/problem+json": { + "schema": { + "$ref": "../definitions/Error.json" + } + } + } + }, + "500": { + "description": "Unexpected error.", + "content": { + "application/problem+json": { + "schema": { + "$ref": "../definitions/Error.json" + } + } + } + } + } + } +} diff --git a/docs/swagger/swagger.json b/docs/swagger/swagger.json index e7663820..8dc21997 100644 --- a/docs/swagger/swagger.json +++ b/docs/swagger/swagger.json @@ -84,6 +84,9 @@ "/rest/v{version}/short-urls/{shortCode}/visits": { "$ref": "paths/v1_short-urls_{shortCode}_visits.json" }, + "/rest/v{version}/tags/{tag}/visits": { + "$ref": "paths/v2_tags_{tag}_visits.json" + }, "/rest/v{version}/mercure-info": { "$ref": "paths/v2_mercure-info.json" diff --git a/module/CLI/config/dependencies.config.php b/module/CLI/config/dependencies.config.php index 0f2e70a5..516bbbd4 100644 --- a/module/CLI/config/dependencies.config.php +++ b/module/CLI/config/dependencies.config.php @@ -11,6 +11,7 @@ use Laminas\ServiceManager\Factory\InvokableFactory; use Shlinkio\Shlink\CLI\Util\GeolocationDbUpdater; use Shlinkio\Shlink\Common\Doctrine\NoDbNameConnectionFactory; use Shlinkio\Shlink\Core\Service; +use Shlinkio\Shlink\Core\Tag\TagService; use Shlinkio\Shlink\Core\Visit; use Shlinkio\Shlink\Installer\Factory\ProcessHelperFactory; use Shlinkio\Shlink\IpGeolocation\GeoLite2\DbUpdater; @@ -78,10 +79,10 @@ return [ Command\Api\DisableKeyCommand::class => [ApiKeyService::class], Command\Api\ListKeysCommand::class => [ApiKeyService::class], - Command\Tag\ListTagsCommand::class => [Service\Tag\TagService::class], - Command\Tag\CreateTagCommand::class => [Service\Tag\TagService::class], - Command\Tag\RenameTagCommand::class => [Service\Tag\TagService::class], - Command\Tag\DeleteTagsCommand::class => [Service\Tag\TagService::class], + Command\Tag\ListTagsCommand::class => [TagService::class], + Command\Tag\CreateTagCommand::class => [TagService::class], + Command\Tag\RenameTagCommand::class => [TagService::class], + Command\Tag\DeleteTagsCommand::class => [TagService::class], Command\Db\CreateDatabaseCommand::class => [ LockFactory::class, diff --git a/module/CLI/src/Command/Tag/CreateTagCommand.php b/module/CLI/src/Command/Tag/CreateTagCommand.php index 5fe56d46..451eb81e 100644 --- a/module/CLI/src/Command/Tag/CreateTagCommand.php +++ b/module/CLI/src/Command/Tag/CreateTagCommand.php @@ -5,7 +5,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI\Command\Tag; use Shlinkio\Shlink\CLI\Util\ExitCodes; -use Shlinkio\Shlink\Core\Service\Tag\TagServiceInterface; +use Shlinkio\Shlink\Core\Tag\TagServiceInterface; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Input\InputOption; diff --git a/module/CLI/src/Command/Tag/DeleteTagsCommand.php b/module/CLI/src/Command/Tag/DeleteTagsCommand.php index 1cebe895..2b3eae14 100644 --- a/module/CLI/src/Command/Tag/DeleteTagsCommand.php +++ b/module/CLI/src/Command/Tag/DeleteTagsCommand.php @@ -5,7 +5,7 @@ declare(strict_types=1); namespace Shlinkio\Shlink\CLI\Command\Tag; use Shlinkio\Shlink\CLI\Util\ExitCodes; -use Shlinkio\Shlink\Core\Service\Tag\TagServiceInterface; +use Shlinkio\Shlink\Core\Tag\TagServiceInterface; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Input\InputOption; diff --git a/module/CLI/src/Command/Tag/ListTagsCommand.php b/module/CLI/src/Command/Tag/ListTagsCommand.php index 0b8f0aa3..11e22a4f 100644 --- a/module/CLI/src/Command/Tag/ListTagsCommand.php +++ b/module/CLI/src/Command/Tag/ListTagsCommand.php @@ -6,8 +6,8 @@ namespace Shlinkio\Shlink\CLI\Command\Tag; use Shlinkio\Shlink\CLI\Util\ExitCodes; use Shlinkio\Shlink\CLI\Util\ShlinkTable; -use Shlinkio\Shlink\Core\Entity\Tag; -use Shlinkio\Shlink\Core\Service\Tag\TagServiceInterface; +use Shlinkio\Shlink\Core\Tag\Model\TagInfo; +use Shlinkio\Shlink\Core\Tag\TagServiceInterface; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; @@ -35,17 +35,20 @@ class ListTagsCommand extends Command protected function execute(InputInterface $input, OutputInterface $output): ?int { - ShlinkTable::fromOutput($output)->render(['Name'], $this->getTagsRows()); + ShlinkTable::fromOutput($output)->render(['Name', 'URLs amount', 'Visits amount'], $this->getTagsRows()); return ExitCodes::EXIT_SUCCESS; } private function getTagsRows(): array { - $tags = $this->tagService->listTags(); + $tags = $this->tagService->tagsInfo(); if (empty($tags)) { - return [['No tags yet']]; + return [['No tags found', '-', '-']]; } - return map($tags, fn (Tag $tag) => [(string) $tag]); + return map( + $tags, + fn (TagInfo $tagInfo) => [(string) $tagInfo->tag(), $tagInfo->shortUrlsCount(), $tagInfo->visitsCount()], + ); } } diff --git a/module/CLI/src/Command/Tag/RenameTagCommand.php b/module/CLI/src/Command/Tag/RenameTagCommand.php index f30bc757..fe42a832 100644 --- a/module/CLI/src/Command/Tag/RenameTagCommand.php +++ b/module/CLI/src/Command/Tag/RenameTagCommand.php @@ -7,7 +7,7 @@ namespace Shlinkio\Shlink\CLI\Command\Tag; use Shlinkio\Shlink\CLI\Util\ExitCodes; use Shlinkio\Shlink\Core\Exception\TagConflictException; use Shlinkio\Shlink\Core\Exception\TagNotFoundException; -use Shlinkio\Shlink\Core\Service\Tag\TagServiceInterface; +use Shlinkio\Shlink\Core\Tag\TagServiceInterface; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; diff --git a/module/CLI/test/Command/Tag/CreateTagCommandTest.php b/module/CLI/test/Command/Tag/CreateTagCommandTest.php index bed087a5..e156cf28 100644 --- a/module/CLI/test/Command/Tag/CreateTagCommandTest.php +++ b/module/CLI/test/Command/Tag/CreateTagCommandTest.php @@ -8,7 +8,7 @@ use Doctrine\Common\Collections\ArrayCollection; use PHPUnit\Framework\TestCase; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\CLI\Command\Tag\CreateTagCommand; -use Shlinkio\Shlink\Core\Service\Tag\TagServiceInterface; +use Shlinkio\Shlink\Core\Tag\TagServiceInterface; use Symfony\Component\Console\Application; use Symfony\Component\Console\Tester\CommandTester; diff --git a/module/CLI/test/Command/Tag/DeleteTagsCommandTest.php b/module/CLI/test/Command/Tag/DeleteTagsCommandTest.php index 060e5aac..27a95de8 100644 --- a/module/CLI/test/Command/Tag/DeleteTagsCommandTest.php +++ b/module/CLI/test/Command/Tag/DeleteTagsCommandTest.php @@ -7,7 +7,7 @@ namespace ShlinkioTest\Shlink\CLI\Command\Tag; use PHPUnit\Framework\TestCase; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\CLI\Command\Tag\DeleteTagsCommand; -use Shlinkio\Shlink\Core\Service\Tag\TagServiceInterface; +use Shlinkio\Shlink\Core\Tag\TagServiceInterface; use Symfony\Component\Console\Application; use Symfony\Component\Console\Tester\CommandTester; diff --git a/module/CLI/test/Command/Tag/ListTagsCommandTest.php b/module/CLI/test/Command/Tag/ListTagsCommandTest.php index f171127c..b6087307 100644 --- a/module/CLI/test/Command/Tag/ListTagsCommandTest.php +++ b/module/CLI/test/Command/Tag/ListTagsCommandTest.php @@ -8,7 +8,8 @@ use PHPUnit\Framework\TestCase; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\CLI\Command\Tag\ListTagsCommand; use Shlinkio\Shlink\Core\Entity\Tag; -use Shlinkio\Shlink\Core\Service\Tag\TagServiceInterface; +use Shlinkio\Shlink\Core\Tag\Model\TagInfo; +use Shlinkio\Shlink\Core\Tag\TagServiceInterface; use Symfony\Component\Console\Application; use Symfony\Component\Console\Tester\CommandTester; @@ -31,28 +32,32 @@ class ListTagsCommandTest extends TestCase /** @test */ public function noTagsPrintsEmptyMessage(): void { - $listTags = $this->tagService->listTags()->willReturn([]); + $tagsInfo = $this->tagService->tagsInfo()->willReturn([]); $this->commandTester->execute([]); $output = $this->commandTester->getDisplay(); - $this->assertStringContainsString('No tags yet', $output); - $listTags->shouldHaveBeenCalled(); + $this->assertStringContainsString('No tags found', $output); + $tagsInfo->shouldHaveBeenCalled(); } /** @test */ public function listOfTagsIsPrinted(): void { - $listTags = $this->tagService->listTags()->willReturn([ - new Tag('foo'), - new Tag('bar'), + $tagsInfo = $this->tagService->tagsInfo()->willReturn([ + new TagInfo(new Tag('foo'), 10, 2), + new TagInfo(new Tag('bar'), 7, 32), ]); $this->commandTester->execute([]); $output = $this->commandTester->getDisplay(); - $this->assertStringContainsString('foo', $output); - $this->assertStringContainsString('bar', $output); - $listTags->shouldHaveBeenCalled(); + $this->assertStringContainsString('| foo', $output); + $this->assertStringContainsString('| bar', $output); + $this->assertStringContainsString('| 10 ', $output); + $this->assertStringContainsString('| 2 ', $output); + $this->assertStringContainsString('| 7 ', $output); + $this->assertStringContainsString('| 32 ', $output); + $tagsInfo->shouldHaveBeenCalled(); } } diff --git a/module/CLI/test/Command/Tag/RenameTagCommandTest.php b/module/CLI/test/Command/Tag/RenameTagCommandTest.php index 59f8d89c..ee499c48 100644 --- a/module/CLI/test/Command/Tag/RenameTagCommandTest.php +++ b/module/CLI/test/Command/Tag/RenameTagCommandTest.php @@ -9,7 +9,7 @@ use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\CLI\Command\Tag\RenameTagCommand; use Shlinkio\Shlink\Core\Entity\Tag; use Shlinkio\Shlink\Core\Exception\TagNotFoundException; -use Shlinkio\Shlink\Core\Service\Tag\TagServiceInterface; +use Shlinkio\Shlink\Core\Tag\TagServiceInterface; use Symfony\Component\Console\Application; use Symfony\Component\Console\Tester\CommandTester; diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index 67d18c40..debf021f 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -28,7 +28,7 @@ return [ Service\ShortUrlService::class => ConfigAbstractFactory::class, Visit\VisitLocator::class => ConfigAbstractFactory::class, Visit\VisitsStatsHelper::class => ConfigAbstractFactory::class, - Service\Tag\TagService::class => ConfigAbstractFactory::class, + Tag\TagService::class => ConfigAbstractFactory::class, Service\ShortUrl\DeleteShortUrlService::class => ConfigAbstractFactory::class, Service\ShortUrl\ShortUrlResolver::class => ConfigAbstractFactory::class, @@ -54,11 +54,15 @@ return [ Options\UrlShortenerOptions::class => ['config.url_shortener'], Service\UrlShortener::class => [Util\UrlValidator::class, 'em', Resolver\PersistenceDomainResolver::class], - Service\VisitsTracker::class => ['em', EventDispatcherInterface::class], + Service\VisitsTracker::class => [ + 'em', + EventDispatcherInterface::class, + 'config.url_shortener.anonymize_remote_addr', + ], Service\ShortUrlService::class => ['em', Service\ShortUrl\ShortUrlResolver::class, Util\UrlValidator::class], Visit\VisitLocator::class => ['em'], Visit\VisitsStatsHelper::class => ['em'], - Service\Tag\TagService::class => ['em'], + Tag\TagService::class => ['em'], Service\ShortUrl\DeleteShortUrlService::class => [ 'em', Options\DeleteShortUrlsOptions::class, diff --git a/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Entity.ShortUrl.php b/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Entity.ShortUrl.php index a4aef29f..871ac113 100644 --- a/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Entity.ShortUrl.php +++ b/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Entity.ShortUrl.php @@ -60,6 +60,7 @@ return static function (ClassMetadata $metadata, array $emConfig): void { ->setJoinTable(determineTableName('short_urls_in_tags', $emConfig)) ->addInverseJoinColumn('tag_id', 'id', true, false, 'CASCADE') ->addJoinColumn('short_url_id', 'id', true, false, 'CASCADE') + ->setOrderBy(['name' => 'ASC']) ->build(); $builder->createManyToOne('domain', Entity\Domain::class) diff --git a/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Entity.Tag.php b/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Entity.Tag.php index 214396bd..97d15758 100644 --- a/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Entity.Tag.php +++ b/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Entity.Tag.php @@ -24,4 +24,6 @@ return static function (ClassMetadata $metadata, array $emConfig): void { $builder->createField('name', Types::STRING) ->unique() ->build(); + + $builder->addInverseManyToMany('shortUrls', Entity\ShortUrl::class, 'tags'); }; diff --git a/module/Core/src/Config/SimplifiedConfigParser.php b/module/Core/src/Config/SimplifiedConfigParser.php index fe4a4192..81f05d14 100644 --- a/module/Core/src/Config/SimplifiedConfigParser.php +++ b/module/Core/src/Config/SimplifiedConfigParser.php @@ -37,6 +37,7 @@ class SimplifiedConfigParser 'mercure_public_hub_url' => ['mercure', 'public_hub_url'], 'mercure_internal_hub_url' => ['mercure', 'internal_hub_url'], 'mercure_jwt_secret' => ['mercure', 'jwt_secret'], + 'anonymize_remote_addr' => ['url_shortener', 'anonymize_remote_addr'], ]; private const SIMPLIFIED_CONFIG_SIDE_EFFECTS = [ 'delete_short_url_threshold' => [ diff --git a/module/Core/src/Entity/ShortUrl.php b/module/Core/src/Entity/ShortUrl.php index 5453d791..0f2811fb 100644 --- a/module/Core/src/Entity/ShortUrl.php +++ b/module/Core/src/Entity/ShortUrl.php @@ -204,10 +204,10 @@ class ShortUrl extends AbstractEntity if ($meta->hasDomain() && $meta->getDomain() !== $this->resolveDomain()) { return false; } - if ($meta->hasValidSince() && ! $meta->getValidSince()->eq($this->validSince)) { + if ($meta->hasValidSince() && ($this->validSince === null || ! $meta->getValidSince()->eq($this->validSince))) { return false; } - if ($meta->hasValidUntil() && ! $meta->getValidUntil()->eq($this->validUntil)) { + if ($meta->hasValidUntil() && ($this->validUntil === null || ! $meta->getValidUntil()->eq($this->validUntil))) { return false; } diff --git a/module/Core/src/Entity/Tag.php b/module/Core/src/Entity/Tag.php index 7530b70a..54c05c56 100644 --- a/module/Core/src/Entity/Tag.php +++ b/module/Core/src/Entity/Tag.php @@ -4,16 +4,19 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Entity; +use Doctrine\Common\Collections; use JsonSerializable; use Shlinkio\Shlink\Common\Entity\AbstractEntity; class Tag extends AbstractEntity implements JsonSerializable { private string $name; + private Collections\Collection $shortUrls; public function __construct(string $name) { $this->name = $name; + $this->shortUrls = new Collections\ArrayCollection(); } public function rename(string $name): void diff --git a/module/Core/src/Entity/Visit.php b/module/Core/src/Entity/Visit.php index e8cbb119..7e6ed060 100644 --- a/module/Core/src/Entity/Visit.php +++ b/module/Core/src/Entity/Visit.php @@ -21,24 +21,24 @@ class Visit extends AbstractEntity implements JsonSerializable private ShortUrl $shortUrl; private ?VisitLocation $visitLocation = null; - public function __construct(ShortUrl $shortUrl, Visitor $visitor, ?Chronos $date = null) + public function __construct(ShortUrl $shortUrl, Visitor $visitor, bool $anonymize = true, ?Chronos $date = null) { $this->shortUrl = $shortUrl; $this->date = $date ?? Chronos::now(); $this->userAgent = $visitor->getUserAgent(); $this->referer = $visitor->getReferer(); - $this->remoteAddr = $this->obfuscateAddress($visitor->getRemoteAddress()); + $this->remoteAddr = $this->processAddress($anonymize, $visitor->getRemoteAddress()); } - private function obfuscateAddress(?string $address): ?string + private function processAddress(bool $anonymize, ?string $address): ?string { - // Localhost addresses do not need to be obfuscated - if ($address === null || $address === IpAddress::LOCALHOST) { + // Localhost addresses do not need to be anonymized + if (! $anonymize || $address === null || $address === IpAddress::LOCALHOST) { return $address; } try { - return (string) IpAddress::fromString($address)->getObfuscatedCopy(); + return (string) IpAddress::fromString($address)->getAnonymizedCopy(); } catch (InvalidArgumentException $e) { return null; } diff --git a/module/Core/src/Paginator/Adapter/AbstractCacheableCountPaginatorAdapter.php b/module/Core/src/Paginator/Adapter/AbstractCacheableCountPaginatorAdapter.php new file mode 100644 index 00000000..cc2a8287 --- /dev/null +++ b/module/Core/src/Paginator/Adapter/AbstractCacheableCountPaginatorAdapter.php @@ -0,0 +1,29 @@ +count !== null) { + return $this->count; + } + + return $this->count = $this->doCount(); + } + + abstract protected function doCount(): int; +} diff --git a/module/Core/src/Paginator/Adapter/VisitsForTagPaginatorAdapter.php b/module/Core/src/Paginator/Adapter/VisitsForTagPaginatorAdapter.php new file mode 100644 index 00000000..e80fbcdd --- /dev/null +++ b/module/Core/src/Paginator/Adapter/VisitsForTagPaginatorAdapter.php @@ -0,0 +1,37 @@ +visitRepository = $visitRepository; + $this->params = $params; + $this->tag = $tag; + } + + public function getItems($offset, $itemCountPerPage): array // phpcs:ignore + { + return $this->visitRepository->findVisitsByTag( + $this->tag, + $this->params->getDateRange(), + $itemCountPerPage, + $offset, + ); + } + + protected function doCount(): int + { + return $this->visitRepository->countVisitsByTag($this->tag, $this->params->getDateRange()); + } +} diff --git a/module/Core/src/Paginator/Adapter/VisitsPaginatorAdapter.php b/module/Core/src/Paginator/Adapter/VisitsPaginatorAdapter.php index 6a42ecbc..404ae309 100644 --- a/module/Core/src/Paginator/Adapter/VisitsPaginatorAdapter.php +++ b/module/Core/src/Paginator/Adapter/VisitsPaginatorAdapter.php @@ -4,19 +4,16 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Paginator\Adapter; -use Laminas\Paginator\Adapter\AdapterInterface; use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Model\VisitsParams; use Shlinkio\Shlink\Core\Repository\VisitRepositoryInterface; -class VisitsPaginatorAdapter implements AdapterInterface +class VisitsPaginatorAdapter extends AbstractCacheableCountPaginatorAdapter { private VisitRepositoryInterface $visitRepository; private ShortUrlIdentifier $identifier; private VisitsParams $params; - private ?int $count = null; - public function __construct( VisitRepositoryInterface $visitRepository, ShortUrlIdentifier $identifier, @@ -38,19 +35,9 @@ class VisitsPaginatorAdapter implements AdapterInterface ); } - public function count(): int + protected function doCount(): int { - // Since a new adapter instance is created every time visits are fetched, it is reasonably safe to internally - // cache the count value. - // The reason it is cached is because the Paginator is actually calling the method twice. - // An inconsistent value could be returned if between the first call and the second one, a new visit is created. - // However, it's almost instant, and then the adapter instance is discarded immediately after. - - if ($this->count !== null) { - return $this->count; - } - - return $this->count = $this->visitRepository->countVisitsByShortCode( + return $this->visitRepository->countVisitsByShortCode( $this->identifier->shortCode(), $this->identifier->domain(), $this->params->getDateRange(), diff --git a/module/Core/src/Repository/TagRepository.php b/module/Core/src/Repository/TagRepository.php index 92328630..05b2481c 100644 --- a/module/Core/src/Repository/TagRepository.php +++ b/module/Core/src/Repository/TagRepository.php @@ -6,6 +6,9 @@ namespace Shlinkio\Shlink\Core\Repository; use Doctrine\ORM\EntityRepository; use Shlinkio\Shlink\Core\Entity\Tag; +use Shlinkio\Shlink\Core\Tag\Model\TagInfo; + +use function Functional\map; class TagRepository extends EntityRepository implements TagRepositoryInterface { @@ -21,4 +24,25 @@ class TagRepository extends EntityRepository implements TagRepositoryInterface return $qb->getQuery()->execute(); } + + /** + * @return TagInfo[] + */ + public function findTagsWithInfo(): array + { + $dql = <<getEntityManager()->createQuery($dql); + + return map( + $query->getResult(), + fn (array $row) => new TagInfo($row['tag'], (int) $row['shortUrlsCount'], (int) $row['visitsCount']), + ); + } } diff --git a/module/Core/src/Repository/TagRepositoryInterface.php b/module/Core/src/Repository/TagRepositoryInterface.php index e253f7a4..37179e21 100644 --- a/module/Core/src/Repository/TagRepositoryInterface.php +++ b/module/Core/src/Repository/TagRepositoryInterface.php @@ -5,8 +5,14 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Repository; use Doctrine\Persistence\ObjectRepository; +use Shlinkio\Shlink\Core\Tag\Model\TagInfo; interface TagRepositoryInterface extends ObjectRepository { public function deleteByName(array $names): int; + + /** + * @return TagInfo[] + */ + public function findTagsWithInfo(): array; } diff --git a/module/Core/src/Repository/VisitRepository.php b/module/Core/src/Repository/VisitRepository.php index 547493c5..b3761c9f 100644 --- a/module/Core/src/Repository/VisitRepository.php +++ b/module/Core/src/Repository/VisitRepository.php @@ -7,13 +7,12 @@ namespace Shlinkio\Shlink\Core\Repository; use Doctrine\ORM\EntityRepository; use Doctrine\ORM\Query\ResultSetMappingBuilder; use Doctrine\ORM\QueryBuilder; -use Shlinkio\Shlink\Common\Doctrine\Type\ChronosDateTimeType; use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Entity\Visit; use Shlinkio\Shlink\Core\Entity\VisitLocation; -use function preg_replace; +use function array_column; use const PHP_INT_MAX; @@ -29,7 +28,7 @@ class VisitRepository extends EntityRepository implements VisitRepositoryInterfa ->from(Visit::class, 'v') ->where($qb->expr()->isNull('v.visitLocation')); - return $this->findVisitsForQuery($qb, $blockSize); + return $this->visitsIterableForQuery($qb, $blockSize); } /** @@ -45,7 +44,7 @@ class VisitRepository extends EntityRepository implements VisitRepositoryInterfa ->andWhere($qb->expr()->eq('vl.isEmpty', ':isEmpty')) ->setParameter('isEmpty', true); - return $this->findVisitsForQuery($qb, $blockSize); + return $this->visitsIterableForQuery($qb, $blockSize); } public function findAllVisits(int $blockSize = self::DEFAULT_BLOCK_SIZE): iterable @@ -54,10 +53,10 @@ class VisitRepository extends EntityRepository implements VisitRepositoryInterfa $qb->select('v') ->from(Visit::class, 'v'); - return $this->findVisitsForQuery($qb, $blockSize); + return $this->visitsIterableForQuery($qb, $blockSize); } - private function findVisitsForQuery(QueryBuilder $qb, int $blockSize): iterable + private function visitsIterableForQuery(QueryBuilder $qb, int $blockSize): iterable { $originalQueryBuilder = $qb->setMaxResults($blockSize) ->orderBy('v.id', 'ASC'); @@ -89,33 +88,101 @@ class VisitRepository extends EntityRepository implements VisitRepositoryInterfa ?int $limit = null, ?int $offset = null ): array { - /** - * @var QueryBuilder $qb - * @var ShortUrl|int $shortUrl - */ - [$qb, $shortUrl] = $this->createVisitsByShortCodeQueryBuilder($shortCode, $domain, $dateRange); + $qb = $this->createVisitsByShortCodeQueryBuilder($shortCode, $domain, $dateRange); + return $this->resolveVisitsWithNativeQuery($qb, $limit, $offset); + } + + public function countVisitsByShortCode(string $shortCode, ?string $domain = null, ?DateRange $dateRange = null): int + { + $qb = $this->createVisitsByShortCodeQueryBuilder($shortCode, $domain, $dateRange); + $qb->select('COUNT(v.id)'); + + return (int) $qb->getQuery()->getSingleScalarResult(); + } + + private function createVisitsByShortCodeQueryBuilder( + string $shortCode, + ?string $domain, + ?DateRange $dateRange + ): QueryBuilder { + /** @var ShortUrlRepositoryInterface $shortUrlRepo */ + $shortUrlRepo = $this->getEntityManager()->getRepository(ShortUrl::class); + $shortUrl = $shortUrlRepo->findOne($shortCode, $domain); + $shortUrlId = $shortUrl !== null ? $shortUrl->getId() : -1; + + // Parameters in this query need to be part of the query itself, as we need to use it a sub-query later + // Since they are not strictly provided by the caller, it's reasonably safe + $qb = $this->getEntityManager()->createQueryBuilder(); + $qb->from(Visit::class, 'v') + ->where($qb->expr()->eq('v.shortUrl', $shortUrlId)); + + // Apply date range filtering + $this->applyDatesInline($qb, $dateRange); + + return $qb; + } + + public function findVisitsByTag( + string $tag, + ?DateRange $dateRange = null, + ?int $limit = null, + ?int $offset = null + ): array { + $qb = $this->createVisitsByTagQueryBuilder($tag, $dateRange); + return $this->resolveVisitsWithNativeQuery($qb, $limit, $offset); + } + + public function countVisitsByTag(string $tag, ?DateRange $dateRange = null): int + { + $qb = $this->createVisitsByTagQueryBuilder($tag, $dateRange); + $qb->select('COUNT(v.id)'); + + return (int) $qb->getQuery()->getSingleScalarResult(); + } + + private function createVisitsByTagQueryBuilder(string $tag, ?DateRange $dateRange = null): QueryBuilder + { + $qb = $this->getEntityManager()->createQueryBuilder(); + $qb->select('s.id') + ->from(ShortUrl::class, 's') + ->join('s.tags', 't') + ->where($qb->expr()->eq('t.name', ':tag')) + ->setParameter('tag', $tag); + + $shortUrlIds = array_column($qb->getQuery()->getArrayResult(), 'id'); + $shortUrlIds[] = '-1'; // Add an invalid ID, in case the list is empty + + // Parameters in this query need to be part of the query itself, as we need to use it a sub-query later + // Since they are not strictly provided by the caller, it's reasonably safe + $qb2 = $this->getEntityManager()->createQueryBuilder(); + $qb2->from(Visit::class, 'v') + ->where($qb2->expr()->in('v.shortUrl', $shortUrlIds)); + + // Apply date range filtering + $this->applyDatesInline($qb2, $dateRange); + + return $qb2; + } + + private function applyDatesInline(QueryBuilder $qb, ?DateRange $dateRange): void + { + if ($dateRange !== null && $dateRange->getStartDate() !== null) { + $qb->andWhere($qb->expr()->gte('v.date', '\'' . $dateRange->getStartDate()->toDateTimeString() . '\'')); + } + if ($dateRange !== null && $dateRange->getEndDate() !== null) { + $qb->andWhere($qb->expr()->lte('v.date', '\'' . $dateRange->getEndDate()->toDateTimeString() . '\'')); + } + } + + private function resolveVisitsWithNativeQuery(QueryBuilder $qb, ?int $limit, ?int $offset): array + { $qb->select('v.id') ->orderBy('v.id', 'DESC') // Falling back to values that will behave as no limit/offset, but will workaround MS SQL not allowing // order on sub-queries without offset ->setMaxResults($limit ?? PHP_INT_MAX) ->setFirstResult($offset ?? 0); - - // FIXME Crappy way to resolve the params into the query. Best option would be to inject the sub-query with - // placeholders and then pass params to the main query - $shortUrlId = $shortUrl instanceof ShortUrl ? $shortUrl->getId() : $shortUrl; - $subQuery = preg_replace('/\?/', $shortUrlId, $qb->getQuery()->getSQL(), 1); - if ($dateRange !== null && $dateRange->getStartDate() !== null) { - $subQuery = preg_replace( - '/\?/', - '\'' . $dateRange->getStartDate()->toDateTimeString() . '\'', - $subQuery, - 1, - ); - } - if ($dateRange !== null && $dateRange->getEndDate() !== null) { - $subQuery = preg_replace('/\?/', '\'' . $dateRange->getEndDate()->toDateTimeString() . '\'', $subQuery, 1); - } + $subQuery = $qb->getQuery()->getSQL(); // A native query builder needs to be used here because DQL and ORM query builders do not accept // sub-queries at "from" and "join" level. @@ -137,40 +204,4 @@ class VisitRepository extends EntityRepository implements VisitRepositoryInterfa return $query->getResult(); } - - public function countVisitsByShortCode(string $shortCode, ?string $domain = null, ?DateRange $dateRange = null): int - { - /** @var QueryBuilder $qb */ - [$qb] = $this->createVisitsByShortCodeQueryBuilder($shortCode, $domain, $dateRange); - $qb->select('COUNT(v.id)'); - - return (int) $qb->getQuery()->getSingleScalarResult(); - } - - private function createVisitsByShortCodeQueryBuilder( - string $shortCode, - ?string $domain, - ?DateRange $dateRange - ): array { - /** @var ShortUrlRepositoryInterface $shortUrlRepo */ - $shortUrlRepo = $this->getEntityManager()->getRepository(ShortUrl::class); - $shortUrl = $shortUrlRepo->findOne($shortCode, $domain) ?? -1; - - $qb = $this->getEntityManager()->createQueryBuilder(); - $qb->from(Visit::class, 'v') - ->where($qb->expr()->eq('v.shortUrl', ':shortUrl')) - ->setParameter('shortUrl', $shortUrl); - - // Apply date range filtering - if ($dateRange !== null && $dateRange->getStartDate() !== null) { - $qb->andWhere($qb->expr()->gte('v.date', ':startDate')) - ->setParameter('startDate', $dateRange->getStartDate(), ChronosDateTimeType::CHRONOS_DATETIME); - } - if ($dateRange !== null && $dateRange->getEndDate() !== null) { - $qb->andWhere($qb->expr()->lte('v.date', ':endDate')) - ->setParameter('endDate', $dateRange->getEndDate(), ChronosDateTimeType::CHRONOS_DATETIME); - } - - return [$qb, $shortUrl]; - } } diff --git a/module/Core/src/Repository/VisitRepositoryInterface.php b/module/Core/src/Repository/VisitRepositoryInterface.php index f9cbc8d9..5a540171 100644 --- a/module/Core/src/Repository/VisitRepositoryInterface.php +++ b/module/Core/src/Repository/VisitRepositoryInterface.php @@ -43,4 +43,16 @@ interface VisitRepositoryInterface extends ObjectRepository ?string $domain = null, ?DateRange $dateRange = null ): int; + + /** + * @return Visit[] + */ + public function findVisitsByTag( + string $tag, + ?DateRange $dateRange = null, + ?int $limit = null, + ?int $offset = null + ): array; + + public function countVisitsByTag(string $tag, ?DateRange $dateRange = null): int; } diff --git a/module/Core/src/Service/VisitsTracker.php b/module/Core/src/Service/VisitsTracker.php index f477681a..e777af76 100644 --- a/module/Core/src/Service/VisitsTracker.php +++ b/module/Core/src/Service/VisitsTracker.php @@ -8,33 +8,39 @@ use Doctrine\ORM; use Laminas\Paginator\Paginator; use Psr\EventDispatcher\EventDispatcherInterface; use Shlinkio\Shlink\Core\Entity\ShortUrl; +use Shlinkio\Shlink\Core\Entity\Tag; use Shlinkio\Shlink\Core\Entity\Visit; use Shlinkio\Shlink\Core\EventDispatcher\ShortUrlVisited; use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; +use Shlinkio\Shlink\Core\Exception\TagNotFoundException; use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Model\Visitor; use Shlinkio\Shlink\Core\Model\VisitsParams; +use Shlinkio\Shlink\Core\Paginator\Adapter\VisitsForTagPaginatorAdapter; use Shlinkio\Shlink\Core\Paginator\Adapter\VisitsPaginatorAdapter; use Shlinkio\Shlink\Core\Repository\ShortUrlRepositoryInterface; -use Shlinkio\Shlink\Core\Repository\VisitRepository; +use Shlinkio\Shlink\Core\Repository\TagRepository; +use Shlinkio\Shlink\Core\Repository\VisitRepositoryInterface; class VisitsTracker implements VisitsTrackerInterface { private ORM\EntityManagerInterface $em; private EventDispatcherInterface $eventDispatcher; + private bool $anonymizeRemoteAddr; - public function __construct(ORM\EntityManagerInterface $em, EventDispatcherInterface $eventDispatcher) - { + public function __construct( + ORM\EntityManagerInterface $em, + EventDispatcherInterface $eventDispatcher, + bool $anonymizeRemoteAddr + ) { $this->em = $em; $this->eventDispatcher = $eventDispatcher; + $this->anonymizeRemoteAddr = $anonymizeRemoteAddr; } - /** - * Tracks a new visit to provided short code from provided visitor - */ public function track(ShortUrl $shortUrl, Visitor $visitor): void { - $visit = new Visit($shortUrl, $visitor); + $visit = new Visit($shortUrl, $visitor, $this->anonymizeRemoteAddr); $this->em->persist($visit); $this->em->flush(); @@ -43,8 +49,6 @@ class VisitsTracker implements VisitsTrackerInterface } /** - * Returns the visits on certain short code - * * @return Visit[]|Paginator * @throws ShortUrlNotFoundException */ @@ -56,7 +60,7 @@ class VisitsTracker implements VisitsTrackerInterface throw ShortUrlNotFoundException::fromNotFound($identifier); } - /** @var VisitRepository $repo */ + /** @var VisitRepositoryInterface $repo */ $repo = $this->em->getRepository(Visit::class); $paginator = new Paginator(new VisitsPaginatorAdapter($repo, $identifier, $params)); $paginator->setItemCountPerPage($params->getItemsPerPage()) @@ -64,4 +68,26 @@ class VisitsTracker implements VisitsTrackerInterface return $paginator; } + + /** + * @return Visit[]|Paginator + * @throws TagNotFoundException + */ + public function visitsForTag(string $tag, VisitsParams $params): Paginator + { + /** @var TagRepository $tagRepo */ + $tagRepo = $this->em->getRepository(Tag::class); + $count = $tagRepo->count(['name' => $tag]); + if ($count === 0) { + throw TagNotFoundException::fromTag($tag); + } + + /** @var VisitRepositoryInterface $repo */ + $repo = $this->em->getRepository(Visit::class); + $paginator = new Paginator(new VisitsForTagPaginatorAdapter($repo, $tag, $params)); + $paginator->setItemCountPerPage($params->getItemsPerPage()) + ->setCurrentPageNumber($params->getPage()); + + return $paginator; + } } diff --git a/module/Core/src/Service/VisitsTrackerInterface.php b/module/Core/src/Service/VisitsTrackerInterface.php index 1ec4e110..2c2759c2 100644 --- a/module/Core/src/Service/VisitsTrackerInterface.php +++ b/module/Core/src/Service/VisitsTrackerInterface.php @@ -8,22 +8,24 @@ use Laminas\Paginator\Paginator; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Entity\Visit; use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; +use Shlinkio\Shlink\Core\Exception\TagNotFoundException; use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Model\Visitor; use Shlinkio\Shlink\Core\Model\VisitsParams; interface VisitsTrackerInterface { - /** - * Tracks a new visit to provided short code from provided visitor - */ public function track(ShortUrl $shortUrl, Visitor $visitor): void; /** - * Returns the visits on certain short code - * * @return Visit[]|Paginator * @throws ShortUrlNotFoundException */ public function info(ShortUrlIdentifier $identifier, VisitsParams $params): Paginator; + + /** + * @return Visit[]|Paginator + * @throws TagNotFoundException + */ + public function visitsForTag(string $tag, VisitsParams $params): Paginator; } diff --git a/module/Core/src/Tag/Model/TagInfo.php b/module/Core/src/Tag/Model/TagInfo.php new file mode 100644 index 00000000..dbc51316 --- /dev/null +++ b/module/Core/src/Tag/Model/TagInfo.php @@ -0,0 +1,46 @@ +tag = $tag; + $this->shortUrlsCount = $shortUrlsCount; + $this->visitsCount = $visitsCount; + } + + public function tag(): Tag + { + return $this->tag; + } + + public function shortUrlsCount(): int + { + return $this->shortUrlsCount; + } + + public function visitsCount(): int + { + return $this->visitsCount; + } + + public function jsonSerialize(): array + { + return [ + 'tag' => $this->tag, + 'shortUrlsCount' => $this->shortUrlsCount, + 'visitsCount' => $this->visitsCount, + ]; + } +} diff --git a/module/Core/src/Service/Tag/TagService.php b/module/Core/src/Tag/TagService.php similarity index 85% rename from module/Core/src/Service/Tag/TagService.php rename to module/Core/src/Tag/TagService.php index b95ddf82..7137e885 100644 --- a/module/Core/src/Service/Tag/TagService.php +++ b/module/Core/src/Tag/TagService.php @@ -2,7 +2,7 @@ declare(strict_types=1); -namespace Shlinkio\Shlink\Core\Service\Tag; +namespace Shlinkio\Shlink\Core\Tag; use Doctrine\Common\Collections\Collection; use Doctrine\ORM; @@ -10,6 +10,8 @@ use Shlinkio\Shlink\Core\Entity\Tag; use Shlinkio\Shlink\Core\Exception\TagConflictException; use Shlinkio\Shlink\Core\Exception\TagNotFoundException; use Shlinkio\Shlink\Core\Repository\TagRepository; +use Shlinkio\Shlink\Core\Repository\TagRepositoryInterface; +use Shlinkio\Shlink\Core\Tag\Model\TagInfo; use Shlinkio\Shlink\Core\Util\TagManagerTrait; class TagService implements TagServiceInterface @@ -25,7 +27,6 @@ class TagService implements TagServiceInterface /** * @return Tag[] - * @throws \UnexpectedValueException */ public function listTags(): array { @@ -34,6 +35,16 @@ class TagService implements TagServiceInterface return $tags; } + /** + * @return TagInfo[] + */ + public function tagsInfo(): array + { + /** @var TagRepositoryInterface $repo */ + $repo = $this->em->getRepository(Tag::class); + return $repo->findTagsWithInfo(); + } + /** * @param string[] $tagNames */ diff --git a/module/Core/src/Service/Tag/TagServiceInterface.php b/module/Core/src/Tag/TagServiceInterface.php similarity index 82% rename from module/Core/src/Service/Tag/TagServiceInterface.php rename to module/Core/src/Tag/TagServiceInterface.php index 16da503c..ed643fc5 100644 --- a/module/Core/src/Service/Tag/TagServiceInterface.php +++ b/module/Core/src/Tag/TagServiceInterface.php @@ -2,12 +2,13 @@ declare(strict_types=1); -namespace Shlinkio\Shlink\Core\Service\Tag; +namespace Shlinkio\Shlink\Core\Tag; use Doctrine\Common\Collections\Collection; use Shlinkio\Shlink\Core\Entity\Tag; use Shlinkio\Shlink\Core\Exception\TagConflictException; use Shlinkio\Shlink\Core\Exception\TagNotFoundException; +use Shlinkio\Shlink\Core\Tag\Model\TagInfo; interface TagServiceInterface { @@ -16,6 +17,11 @@ interface TagServiceInterface */ public function listTags(): array; + /** + * @return TagInfo[] + */ + public function tagsInfo(): array; + /** * @param string[] $tagNames */ diff --git a/module/Core/test-db/Repository/TagRepositoryTest.php b/module/Core/test-db/Repository/TagRepositoryTest.php index 94e38f53..8e1a11ef 100644 --- a/module/Core/test-db/Repository/TagRepositoryTest.php +++ b/module/Core/test-db/Repository/TagRepositoryTest.php @@ -4,13 +4,21 @@ declare(strict_types=1); namespace ShlinkioTest\Shlink\Core\Repository; +use Doctrine\Common\Collections\ArrayCollection; +use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Entity\Tag; +use Shlinkio\Shlink\Core\Entity\Visit; +use Shlinkio\Shlink\Core\Model\Visitor; use Shlinkio\Shlink\Core\Repository\TagRepository; use Shlinkio\Shlink\TestUtils\DbTest\DatabaseTestCase; +use function array_chunk; + class TagRepositoryTest extends DatabaseTestCase { protected const ENTITIES_TO_EMPTY = [ + Visit::class, + ShortUrl::class, Tag::class, ]; @@ -40,4 +48,53 @@ class TagRepositoryTest extends DatabaseTestCase $this->assertEquals(2, $this->repo->deleteByName($toDelete)); } + + /** @test */ + public function properTagsInfoIsReturned(): void + { + $names = ['foo', 'bar', 'baz', 'another']; + $tags = []; + foreach ($names as $name) { + $tag = new Tag($name); + $tags[] = $tag; + $this->getEntityManager()->persist($tag); + } + + [$firstUrlTags] = array_chunk($tags, 3); + $secondUrlTags = [$tags[0]]; + + $shortUrl = new ShortUrl(''); + $shortUrl->setTags(new ArrayCollection($firstUrlTags)); + $this->getEntityManager()->persist($shortUrl); + $this->getEntityManager()->persist(new Visit($shortUrl, Visitor::emptyInstance())); + $this->getEntityManager()->persist(new Visit($shortUrl, Visitor::emptyInstance())); + $this->getEntityManager()->persist(new Visit($shortUrl, Visitor::emptyInstance())); + + $shortUrl2 = new ShortUrl(''); + $shortUrl2->setTags(new ArrayCollection($secondUrlTags)); + $this->getEntityManager()->persist($shortUrl2); + $this->getEntityManager()->persist(new Visit($shortUrl2, Visitor::emptyInstance())); + + $this->getEntityManager()->flush(); + + $result = $this->repo->findTagsWithInfo(); + + $this->assertCount(4, $result); + $this->assertEquals( + ['tag' => $tags[3], 'shortUrlsCount' => 0, 'visitsCount' => 0], + $result[0]->jsonSerialize(), + ); + $this->assertEquals( + ['tag' => $tags[1], 'shortUrlsCount' => 1, 'visitsCount' => 3], + $result[1]->jsonSerialize(), + ); + $this->assertEquals( + ['tag' => $tags[2], 'shortUrlsCount' => 1, 'visitsCount' => 3], + $result[2]->jsonSerialize(), + ); + $this->assertEquals( + ['tag' => $tags[0], 'shortUrlsCount' => 2, 'visitsCount' => 4], + $result[3]->jsonSerialize(), + ); + } } diff --git a/module/Core/test-db/Repository/VisitRepositoryTest.php b/module/Core/test-db/Repository/VisitRepositoryTest.php index 034b15f9..529a5ae0 100644 --- a/module/Core/test-db/Repository/VisitRepositoryTest.php +++ b/module/Core/test-db/Repository/VisitRepositoryTest.php @@ -5,9 +5,11 @@ declare(strict_types=1); namespace ShlinkioTest\Shlink\Core\Repository; use Cake\Chronos\Chronos; +use Doctrine\Common\Collections\ArrayCollection; use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\Entity\Domain; use Shlinkio\Shlink\Core\Entity\ShortUrl; +use Shlinkio\Shlink\Core\Entity\Tag; use Shlinkio\Shlink\Core\Entity\Visit; use Shlinkio\Shlink\Core\Entity\VisitLocation; use Shlinkio\Shlink\Core\Model\ShortUrlMeta; @@ -27,6 +29,7 @@ class VisitRepositoryTest extends DatabaseTestCase Visit::class, ShortUrl::class, Domain::class, + Tag::class, ]; private VisitRepository $repo; @@ -125,33 +128,99 @@ class VisitRepositoryTest extends DatabaseTestCase ))); } - private function createShortUrlsAndVisits(): array + /** @test */ + public function findVisitsByTagReturnsProperData(): void + { + $foo = new Tag('foo'); + $this->getEntityManager()->persist($foo); + + /** @var ShortUrl $shortUrl */ + [,, $shortUrl] = $this->createShortUrlsAndVisits(false); + /** @var ShortUrl $shortUrl2 */ + [,, $shortUrl2] = $this->createShortUrlsAndVisits(false); + /** @var ShortUrl $shortUrl3 */ + [,, $shortUrl3] = $this->createShortUrlsAndVisits(false); + + $shortUrl->setTags(new ArrayCollection([$foo])); + $shortUrl2->setTags(new ArrayCollection([$foo])); + $shortUrl3->setTags(new ArrayCollection([$foo])); + + $this->getEntityManager()->flush(); + + $this->assertCount(0, $this->repo->findVisitsByTag('invalid')); + $this->assertCount(18, $this->repo->findVisitsByTag((string) $foo)); + $this->assertCount(6, $this->repo->findVisitsByTag((string) $foo, new DateRange( + Chronos::parse('2016-01-02'), + Chronos::parse('2016-01-03'), + ))); + $this->assertCount(12, $this->repo->findVisitsByTag((string) $foo, new DateRange( + Chronos::parse('2016-01-03'), + ))); + } + + /** @test */ + public function countVisitsByTagReturnsProperData(): void + { + $foo = new Tag('foo'); + $this->getEntityManager()->persist($foo); + + /** @var ShortUrl $shortUrl */ + [,, $shortUrl] = $this->createShortUrlsAndVisits(false); + /** @var ShortUrl $shortUrl2 */ + [,, $shortUrl2] = $this->createShortUrlsAndVisits(false); + + $shortUrl->setTags(new ArrayCollection([$foo])); + $shortUrl2->setTags(new ArrayCollection([$foo])); + + $this->getEntityManager()->flush(); + + $this->assertEquals(0, $this->repo->countVisitsByTag('invalid')); + $this->assertEquals(12, $this->repo->countVisitsByTag((string) $foo)); + $this->assertEquals(4, $this->repo->countVisitsByTag((string) $foo, new DateRange( + Chronos::parse('2016-01-02'), + Chronos::parse('2016-01-03'), + ))); + $this->assertEquals(8, $this->repo->countVisitsByTag((string) $foo, new DateRange( + Chronos::parse('2016-01-03'), + ))); + } + + private function createShortUrlsAndVisits(bool $withDomain = true): array { $shortUrl = new ShortUrl(''); $domain = 'example.com'; $shortCode = $shortUrl->getShortCode(); - $shortUrlWithDomain = new ShortUrl('', ShortUrlMeta::fromRawData([ - 'customSlug' => $shortCode, - 'domain' => $domain, - ])); - $this->getEntityManager()->persist($shortUrl); - $this->getEntityManager()->persist($shortUrlWithDomain); for ($i = 0; $i < 6; $i++) { - $visit = new Visit($shortUrl, Visitor::emptyInstance(), Chronos::parse(sprintf('2016-01-0%s', $i + 1))); - $this->getEntityManager()->persist($visit); - } - for ($i = 0; $i < 3; $i++) { $visit = new Visit( - $shortUrlWithDomain, + $shortUrl, Visitor::emptyInstance(), + true, Chronos::parse(sprintf('2016-01-0%s', $i + 1)), ); $this->getEntityManager()->persist($visit); } - $this->getEntityManager()->flush(); - return [$shortCode, $domain]; + if ($withDomain) { + $shortUrlWithDomain = new ShortUrl('', ShortUrlMeta::fromRawData([ + 'customSlug' => $shortCode, + 'domain' => $domain, + ])); + $this->getEntityManager()->persist($shortUrlWithDomain); + + for ($i = 0; $i < 3; $i++) { + $visit = new Visit( + $shortUrlWithDomain, + Visitor::emptyInstance(), + true, + Chronos::parse(sprintf('2016-01-0%s', $i + 1)), + ); + $this->getEntityManager()->persist($visit); + } + $this->getEntityManager()->flush(); + } + + return [$shortCode, $domain, $shortUrl]; } } diff --git a/module/Core/test/Config/SimplifiedConfigParserTest.php b/module/Core/test/Config/SimplifiedConfigParserTest.php index 6bc6f1f7..3700b042 100644 --- a/module/Core/test/Config/SimplifiedConfigParserTest.php +++ b/module/Core/test/Config/SimplifiedConfigParserTest.php @@ -64,6 +64,7 @@ class SimplifiedConfigParserTest extends TestCase 'mercure_public_hub_url' => 'public_url', 'mercure_internal_hub_url' => 'internal_url', 'mercure_jwt_secret' => 'super_secret_value', + 'anonymize_remote_addr' => false, ]; $expected = [ 'app_options' => [ @@ -92,6 +93,7 @@ class SimplifiedConfigParserTest extends TestCase 'https://third-party.io/foo', ], 'default_short_codes_length' => 8, + 'anonymize_remote_addr' => false, ], 'delete_short_urls' => [ diff --git a/module/Core/test/Entity/ShortUrlTest.php b/module/Core/test/Entity/ShortUrlTest.php index e410dedb..07308580 100644 --- a/module/Core/test/Entity/ShortUrlTest.php +++ b/module/Core/test/Entity/ShortUrlTest.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace ShlinkioTest\Shlink\Core\Entity; +use Cake\Chronos\Chronos; use PHPUnit\Framework\TestCase; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Exception\ShortCodeCannotBeRegeneratedException; @@ -74,4 +75,38 @@ class ShortUrlTest extends TestCase yield [null, DEFAULT_SHORT_CODES_LENGTH]; yield from map(range(4, 10), fn (int $value) => [$value, $value]); } + + /** + * @test + * @dataProvider provideCriteriaToMatch + */ + public function criteriaIsMatchedWhenDatesMatch(ShortUrl $shortUrl, ShortUrlMeta $meta, bool $expected): void + { + $this->assertEquals($expected, $shortUrl->matchesCriteria($meta, [])); + } + + public function provideCriteriaToMatch(): iterable + { + $start = Chronos::parse('2020-03-05 20:18:30'); + $end = Chronos::parse('2021-03-05 20:18:30'); + + yield [new ShortUrl('foo'), ShortUrlMeta::fromRawData(['validSince' => $start]), false]; + yield [new ShortUrl('foo'), ShortUrlMeta::fromRawData(['validUntil' => $end]), false]; + yield [new ShortUrl('foo'), ShortUrlMeta::fromRawData(['validSince' => $start, 'validUntil' => $end]), false]; + yield [ + new ShortUrl('foo', ShortUrlMeta::fromRawData(['validSince' => $start])), + ShortUrlMeta::fromRawData(['validSince' => $start]), + true, + ]; + yield [ + new ShortUrl('foo', ShortUrlMeta::fromRawData(['validUntil' => $end])), + ShortUrlMeta::fromRawData(['validUntil' => $end]), + true, + ]; + yield [ + new ShortUrl('foo', ShortUrlMeta::fromRawData(['validUntil' => $end, 'validSince' => $start])), + ShortUrlMeta::fromRawData(['validUntil' => $end, 'validSince' => $start]), + true, + ]; + } } diff --git a/module/Core/test/Entity/VisitTest.php b/module/Core/test/Entity/VisitTest.php index 9af71a09..73e41f12 100644 --- a/module/Core/test/Entity/VisitTest.php +++ b/module/Core/test/Entity/VisitTest.php @@ -6,6 +6,7 @@ namespace ShlinkioTest\Shlink\Core\Entity; use Cake\Chronos\Chronos; use PHPUnit\Framework\TestCase; +use Shlinkio\Shlink\Common\Util\IpAddress; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Entity\Visit; use Shlinkio\Shlink\Core\Model\Visitor; @@ -18,7 +19,7 @@ class VisitTest extends TestCase */ public function isProperlyJsonSerialized(?Chronos $date): void { - $visit = new Visit(new ShortUrl(''), new Visitor('Chrome', 'some site', '1.2.3.4'), $date); + $visit = new Visit(new ShortUrl(''), new Visitor('Chrome', 'some site', '1.2.3.4'), true, $date); $this->assertEquals([ 'referer' => 'some site', @@ -33,4 +34,25 @@ class VisitTest extends TestCase yield 'null date' => [null]; yield 'not null date' => [Chronos::now()->subDays(10)]; } + + /** + * @test + * @dataProvider provideAddresses + */ + public function addressIsAnonymizedWhenRequested(bool $anonymize, ?string $address, ?string $expectedAddress): void + { + $visit = new Visit(new ShortUrl(''), new Visitor('Chrome', 'some site', $address), $anonymize); + + $this->assertEquals($expectedAddress, $visit->getRemoteAddr()); + } + + public function provideAddresses(): iterable + { + yield 'anonymized null address' => [true, null, null]; + yield 'non-anonymized null address' => [false, null, null]; + yield 'anonymized localhost' => [true, IpAddress::LOCALHOST, IpAddress::LOCALHOST]; + yield 'non-anonymized localhost' => [false, IpAddress::LOCALHOST, IpAddress::LOCALHOST]; + yield 'anonymized regular address' => [true, '1.2.3.4', '1.2.3.0']; + yield 'non-anonymized regular address' => [false, '1.2.3.4', '1.2.3.4']; + } } diff --git a/module/Core/test/Paginator/Adapter/VisitsForTagPaginatorAdapterTest.php b/module/Core/test/Paginator/Adapter/VisitsForTagPaginatorAdapterTest.php new file mode 100644 index 00000000..e4418c5b --- /dev/null +++ b/module/Core/test/Paginator/Adapter/VisitsForTagPaginatorAdapterTest.php @@ -0,0 +1,52 @@ +repo = $this->prophesize(VisitRepositoryInterface::class); + $this->adapter = new VisitsForTagPaginatorAdapter($this->repo->reveal(), 'foo', VisitsParams::fromRawData([])); + } + + /** @test */ + public function repoIsCalledEveryTimeItemsAreFetched(): void + { + $count = 3; + $limit = 1; + $offset = 5; + $findVisits = $this->repo->findVisitsByTag('foo', new DateRange(), $limit, $offset)->willReturn([]); + + for ($i = 0; $i < $count; $i++) { + $this->adapter->getItems($offset, $limit); + } + + $findVisits->shouldHaveBeenCalledTimes($count); + } + + /** @test */ + public function repoIsCalledOnlyOnceForCount(): void + { + $count = 3; + $countVisits = $this->repo->countVisitsByTag('foo', new DateRange())->willReturn(3); + + for ($i = 0; $i < $count; $i++) { + $this->adapter->count(); + } + + $countVisits->shouldHaveBeenCalledOnce(); + } +} diff --git a/module/Core/test/Paginator/Adapter/VisitsPaginatorAdapterTest.php b/module/Core/test/Paginator/Adapter/VisitsPaginatorAdapterTest.php new file mode 100644 index 00000000..744582b7 --- /dev/null +++ b/module/Core/test/Paginator/Adapter/VisitsPaginatorAdapterTest.php @@ -0,0 +1,57 @@ +repo = $this->prophesize(VisitRepositoryInterface::class); + $this->adapter = new VisitsPaginatorAdapter( + $this->repo->reveal(), + new ShortUrlIdentifier(''), + VisitsParams::fromRawData([]), + ); + } + + /** @test */ + public function repoIsCalledEveryTimeItemsAreFetched(): void + { + $count = 3; + $limit = 1; + $offset = 5; + $findVisits = $this->repo->findVisitsByShortCode('', null, new DateRange(), $limit, $offset)->willReturn([]); + + for ($i = 0; $i < $count; $i++) { + $this->adapter->getItems($offset, $limit); + } + + $findVisits->shouldHaveBeenCalledTimes($count); + } + + /** @test */ + public function repoIsCalledOnlyOnceForCount(): void + { + $count = 3; + $countVisits = $this->repo->countVisitsByShortCode('', null, new DateRange())->willReturn(3); + + for ($i = 0; $i < $count; $i++) { + $this->adapter->count(); + } + + $countVisits->shouldHaveBeenCalledOnce(); + } +} diff --git a/module/Core/test/Service/Tag/TagServiceTest.php b/module/Core/test/Service/Tag/TagServiceTest.php index b8c9d59b..c031e51f 100644 --- a/module/Core/test/Service/Tag/TagServiceTest.php +++ b/module/Core/test/Service/Tag/TagServiceTest.php @@ -5,7 +5,6 @@ declare(strict_types=1); namespace ShlinkioTest\Shlink\Core\Service\Tag; use Doctrine\ORM\EntityManagerInterface; -use Doctrine\ORM\EntityRepository; use PHPUnit\Framework\TestCase; use Prophecy\Argument; use Prophecy\Prophecy\ObjectProphecy; @@ -13,16 +12,21 @@ use Shlinkio\Shlink\Core\Entity\Tag; use Shlinkio\Shlink\Core\Exception\TagConflictException; use Shlinkio\Shlink\Core\Exception\TagNotFoundException; use Shlinkio\Shlink\Core\Repository\TagRepository; -use Shlinkio\Shlink\Core\Service\Tag\TagService; +use Shlinkio\Shlink\Core\Tag\Model\TagInfo; +use Shlinkio\Shlink\Core\Tag\TagService; class TagServiceTest extends TestCase { private TagService $service; private ObjectProphecy $em; + private ObjectProphecy $repo; public function setUp(): void { $this->em = $this->prophesize(EntityManagerInterface::class); + $this->repo = $this->prophesize(TagRepository::class); + $this->em->getRepository(Tag::class)->willReturn($this->repo->reveal())->shouldBeCalled(); + $this->service = new TagService($this->em->reveal()); } @@ -31,36 +35,41 @@ class TagServiceTest extends TestCase { $expected = [new Tag('foo'), new Tag('bar')]; - $repo = $this->prophesize(EntityRepository::class); - $find = $repo->findBy(Argument::cetera())->willReturn($expected); - $getRepo = $this->em->getRepository(Tag::class)->willReturn($repo->reveal()); + $find = $this->repo->findBy(Argument::cetera())->willReturn($expected); $result = $this->service->listTags(); $this->assertEquals($expected, $result); $find->shouldHaveBeenCalled(); - $getRepo->shouldHaveBeenCalled(); + } + + /** @test */ + public function tagsInfoDelegatesOnRepository(): void + { + $expected = [new TagInfo(new Tag('foo'), 1, 1), new TagInfo(new Tag('bar'), 3, 10)]; + + $find = $this->repo->findTagsWithInfo()->willReturn($expected); + + $result = $this->service->tagsInfo(); + + $this->assertEquals($expected, $result); + $find->shouldHaveBeenCalled(); } /** @test */ public function deleteTagsDelegatesOnRepository(): void { - $repo = $this->prophesize(TagRepository::class); - $delete = $repo->deleteByName(['foo', 'bar'])->willReturn(4); - $getRepo = $this->em->getRepository(Tag::class)->willReturn($repo->reveal()); + $delete = $this->repo->deleteByName(['foo', 'bar'])->willReturn(4); $this->service->deleteTags(['foo', 'bar']); $delete->shouldHaveBeenCalled(); - $getRepo->shouldHaveBeenCalled(); } /** @test */ public function createTagsPersistsEntities(): void { - $repo = $this->prophesize(TagRepository::class); - $find = $repo->findOneBy(Argument::cetera())->willReturn(new Tag('foo')); - $getRepo = $this->em->getRepository(Tag::class)->willReturn($repo->reveal()); + $find = $this->repo->findOneBy(Argument::cetera())->willReturn(new Tag('foo')); $persist = $this->em->persist(Argument::type(Tag::class))->willReturn(null); $flush = $this->em->flush()->willReturn(null); @@ -68,7 +77,6 @@ class TagServiceTest extends TestCase $this->assertCount(2, $result); $find->shouldHaveBeenCalled(); - $getRepo->shouldHaveBeenCalled(); $persist->shouldHaveBeenCalledTimes(2); $flush->shouldHaveBeenCalled(); } @@ -76,12 +84,9 @@ class TagServiceTest extends TestCase /** @test */ public function renameInvalidTagThrowsException(): void { - $repo = $this->prophesize(TagRepository::class); - $find = $repo->findOneBy(Argument::cetera())->willReturn(null); - $getRepo = $this->em->getRepository(Tag::class)->willReturn($repo->reveal()); + $find = $this->repo->findOneBy(Argument::cetera())->willReturn(null); $find->shouldBeCalled(); - $getRepo->shouldBeCalled(); $this->expectException(TagNotFoundException::class); $this->service->renameTag('foo', 'bar'); @@ -95,10 +100,8 @@ class TagServiceTest extends TestCase { $expected = new Tag('foo'); - $repo = $this->prophesize(TagRepository::class); - $find = $repo->findOneBy(Argument::cetera())->willReturn($expected); - $countTags = $repo->count(Argument::cetera())->willReturn($count); - $getRepo = $this->em->getRepository(Tag::class)->willReturn($repo->reveal()); + $find = $this->repo->findOneBy(Argument::cetera())->willReturn($expected); + $countTags = $this->repo->count(Argument::cetera())->willReturn($count); $flush = $this->em->flush()->willReturn(null); $tag = $this->service->renameTag($oldName, $newName); @@ -106,7 +109,6 @@ class TagServiceTest extends TestCase $this->assertSame($expected, $tag); $this->assertEquals($newName, (string) $tag); $find->shouldHaveBeenCalled(); - $getRepo->shouldHaveBeenCalled(); $flush->shouldHaveBeenCalled(); $countTags->shouldHaveBeenCalledTimes($count > 0 ? 0 : 1); } @@ -120,14 +122,11 @@ class TagServiceTest extends TestCase /** @test */ public function renameTagToAnExistingNameThrowsException(): void { - $repo = $this->prophesize(TagRepository::class); - $find = $repo->findOneBy(Argument::cetera())->willReturn(new Tag('foo')); - $countTags = $repo->count(Argument::cetera())->willReturn(1); - $getRepo = $this->em->getRepository(Tag::class)->willReturn($repo->reveal()); + $find = $this->repo->findOneBy(Argument::cetera())->willReturn(new Tag('foo')); + $countTags = $this->repo->count(Argument::cetera())->willReturn(1); $flush = $this->em->flush(Argument::any())->willReturn(null); $find->shouldBeCalled(); - $getRepo->shouldBeCalled(); $countTags->shouldBeCalled(); $flush->shouldNotBeCalled(); $this->expectException(TagConflictException::class); diff --git a/module/Core/test/Service/VisitsTrackerTest.php b/module/Core/test/Service/VisitsTrackerTest.php index 9028d2c7..5893b952 100644 --- a/module/Core/test/Service/VisitsTrackerTest.php +++ b/module/Core/test/Service/VisitsTrackerTest.php @@ -6,20 +6,22 @@ namespace ShlinkioTest\Shlink\Core\Service; use Doctrine\ORM\EntityManager; use Laminas\Stdlib\ArrayUtils; -use PHPUnit\Framework\Assert; use PHPUnit\Framework\TestCase; use Prophecy\Argument; use Prophecy\Prophecy\ObjectProphecy; use Psr\EventDispatcher\EventDispatcherInterface; use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\Entity\ShortUrl; +use Shlinkio\Shlink\Core\Entity\Tag; use Shlinkio\Shlink\Core\Entity\Visit; use Shlinkio\Shlink\Core\EventDispatcher\ShortUrlVisited; use Shlinkio\Shlink\Core\Exception\ShortUrlNotFoundException; +use Shlinkio\Shlink\Core\Exception\TagNotFoundException; use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Model\Visitor; use Shlinkio\Shlink\Core\Model\VisitsParams; use Shlinkio\Shlink\Core\Repository\ShortUrlRepositoryInterface; +use Shlinkio\Shlink\Core\Repository\TagRepository; use Shlinkio\Shlink\Core\Repository\VisitRepository; use Shlinkio\Shlink\Core\Service\VisitsTracker; @@ -37,7 +39,7 @@ class VisitsTrackerTest extends TestCase $this->em = $this->prophesize(EntityManager::class); $this->eventDispatcher = $this->prophesize(EventDispatcherInterface::class); - $this->visitsTracker = new VisitsTracker($this->em->reveal(), $this->eventDispatcher->reveal()); + $this->visitsTracker = new VisitsTracker($this->em->reveal(), $this->eventDispatcher->reveal(), true); } /** @test */ @@ -53,25 +55,6 @@ class VisitsTrackerTest extends TestCase $this->eventDispatcher->dispatch(Argument::type(ShortUrlVisited::class))->shouldHaveBeenCalled(); } - /** @test */ - public function trackedIpAddressGetsObfuscated(): void - { - $shortCode = '123ABC'; - - $this->em->persist(Argument::any())->will(function ($args) { - /** @var Visit $visit */ - $visit = $args[0]; - Assert::assertEquals('4.3.2.0', $visit->getRemoteAddr()); - $visit->setId('1'); - return $visit; - })->shouldBeCalledOnce(); - $this->em->flush()->shouldBeCalledOnce(); - - $this->visitsTracker->track(new ShortUrl($shortCode), new Visitor('', '', '4.3.2.1')); - - $this->eventDispatcher->dispatch(Argument::type(ShortUrlVisited::class))->shouldHaveBeenCalled(); - } - /** @test */ public function infoReturnsVisitsForCertainShortCode(): void { @@ -105,4 +88,40 @@ class VisitsTrackerTest extends TestCase $this->visitsTracker->info(new ShortUrlIdentifier($shortCode), new VisitsParams()); } + + /** @test */ + public function throwsExceptionWhenRequestingVisitsForInvalidTag(): void + { + $tag = 'foo'; + $repo = $this->prophesize(TagRepository::class); + $count = $repo->count(['name' => $tag])->willReturn(0); + $getRepo = $this->em->getRepository(Tag::class)->willReturn($repo->reveal()); + + $this->expectException(TagNotFoundException::class); + $count->shouldBeCalledOnce(); + $getRepo->shouldBeCalledOnce(); + + $this->visitsTracker->visitsForTag($tag, new VisitsParams()); + } + + /** @test */ + public function visitsForTagAreReturnedAsExpected(): void + { + $tag = 'foo'; + $repo = $this->prophesize(TagRepository::class); + $count = $repo->count(['name' => $tag])->willReturn(1); + $getRepo = $this->em->getRepository(Tag::class)->willReturn($repo->reveal()); + + $list = map(range(0, 1), fn () => new Visit(new ShortUrl(''), Visitor::emptyInstance())); + $repo2 = $this->prophesize(VisitRepository::class); + $repo2->findVisitsByTag($tag, Argument::type(DateRange::class), 1, 0)->willReturn($list); + $repo2->countVisitsByTag($tag, Argument::type(DateRange::class))->willReturn(1); + $this->em->getRepository(Visit::class)->willReturn($repo2->reveal())->shouldBeCalledOnce(); + + $paginator = $this->visitsTracker->visitsForTag($tag, new VisitsParams()); + + $this->assertEquals($list, ArrayUtils::iteratorToArray($paginator->getCurrentItems())); + $count->shouldHaveBeenCalledOnce(); + $getRepo->shouldHaveBeenCalledOnce(); + } } diff --git a/module/Rest/config/dependencies.config.php b/module/Rest/config/dependencies.config.php index bd347897..258404ef 100644 --- a/module/Rest/config/dependencies.config.php +++ b/module/Rest/config/dependencies.config.php @@ -10,6 +10,7 @@ use Mezzio\Router\Middleware\ImplicitOptionsMiddleware; use Shlinkio\Shlink\Common\Mercure\LcobucciJwtProvider; use Shlinkio\Shlink\Core\Options\AppOptions; use Shlinkio\Shlink\Core\Service; +use Shlinkio\Shlink\Core\Tag\TagService; use Shlinkio\Shlink\Core\Visit; use Shlinkio\Shlink\Rest\Service\ApiKeyService; @@ -29,6 +30,7 @@ return [ Action\ShortUrl\ListShortUrlsAction::class => ConfigAbstractFactory::class, Action\ShortUrl\EditShortUrlTagsAction::class => ConfigAbstractFactory::class, Action\Visit\ShortUrlVisitsAction::class => ConfigAbstractFactory::class, + Action\Visit\TagVisitsAction::class => ConfigAbstractFactory::class, Action\Visit\GlobalVisitsAction::class => ConfigAbstractFactory::class, Action\Tag\ListTagsAction::class => ConfigAbstractFactory::class, Action\Tag\DeleteTagsAction::class => ConfigAbstractFactory::class, @@ -62,13 +64,14 @@ return [ 'config.url_shortener.domain', ], Action\Visit\ShortUrlVisitsAction::class => [Service\VisitsTracker::class], + Action\Visit\TagVisitsAction::class => [Service\VisitsTracker::class], Action\Visit\GlobalVisitsAction::class => [Visit\VisitsStatsHelper::class], Action\ShortUrl\ListShortUrlsAction::class => [Service\ShortUrlService::class, 'config.url_shortener.domain'], Action\ShortUrl\EditShortUrlTagsAction::class => [Service\ShortUrlService::class], - Action\Tag\ListTagsAction::class => [Service\Tag\TagService::class], - Action\Tag\DeleteTagsAction::class => [Service\Tag\TagService::class], - Action\Tag\CreateTagsAction::class => [Service\Tag\TagService::class], - Action\Tag\UpdateTagAction::class => [Service\Tag\TagService::class], + Action\Tag\ListTagsAction::class => [TagService::class], + Action\Tag\DeleteTagsAction::class => [TagService::class], + Action\Tag\CreateTagsAction::class => [TagService::class], + Action\Tag\UpdateTagAction::class => [TagService::class], Middleware\ShortUrl\DropDefaultDomainFromRequestMiddleware::class => ['config.url_shortener.domain.hostname'], Middleware\ShortUrl\DefaultShortCodesLengthMiddleware::class => [ diff --git a/module/Rest/config/routes.config.php b/module/Rest/config/routes.config.php index d2795971..0bde3da0 100644 --- a/module/Rest/config/routes.config.php +++ b/module/Rest/config/routes.config.php @@ -27,6 +27,7 @@ return [ // Visits Action\Visit\ShortUrlVisitsAction::getRouteDef([$dropDomainMiddleware]), + Action\Visit\TagVisitsAction::getRouteDef(), Action\Visit\GlobalVisitsAction::getRouteDef(), // Tags diff --git a/module/Rest/src/Action/Tag/CreateTagsAction.php b/module/Rest/src/Action/Tag/CreateTagsAction.php index c481b463..08f617c2 100644 --- a/module/Rest/src/Action/Tag/CreateTagsAction.php +++ b/module/Rest/src/Action/Tag/CreateTagsAction.php @@ -7,7 +7,7 @@ namespace Shlinkio\Shlink\Rest\Action\Tag; use Laminas\Diactoros\Response\JsonResponse; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; -use Shlinkio\Shlink\Core\Service\Tag\TagServiceInterface; +use Shlinkio\Shlink\Core\Tag\TagServiceInterface; use Shlinkio\Shlink\Rest\Action\AbstractRestAction; class CreateTagsAction extends AbstractRestAction diff --git a/module/Rest/src/Action/Tag/DeleteTagsAction.php b/module/Rest/src/Action/Tag/DeleteTagsAction.php index 5002eba0..f38c443a 100644 --- a/module/Rest/src/Action/Tag/DeleteTagsAction.php +++ b/module/Rest/src/Action/Tag/DeleteTagsAction.php @@ -7,7 +7,7 @@ namespace Shlinkio\Shlink\Rest\Action\Tag; use Laminas\Diactoros\Response\EmptyResponse; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; -use Shlinkio\Shlink\Core\Service\Tag\TagServiceInterface; +use Shlinkio\Shlink\Core\Tag\TagServiceInterface; use Shlinkio\Shlink\Rest\Action\AbstractRestAction; class DeleteTagsAction extends AbstractRestAction diff --git a/module/Rest/src/Action/Tag/ListTagsAction.php b/module/Rest/src/Action/Tag/ListTagsAction.php index 7211bce6..0832f17c 100644 --- a/module/Rest/src/Action/Tag/ListTagsAction.php +++ b/module/Rest/src/Action/Tag/ListTagsAction.php @@ -7,9 +7,12 @@ namespace Shlinkio\Shlink\Rest\Action\Tag; use Laminas\Diactoros\Response\JsonResponse; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; -use Shlinkio\Shlink\Core\Service\Tag\TagServiceInterface; +use Shlinkio\Shlink\Core\Tag\Model\TagInfo; +use Shlinkio\Shlink\Core\Tag\TagServiceInterface; use Shlinkio\Shlink\Rest\Action\AbstractRestAction; +use function Functional\map; + class ListTagsAction extends AbstractRestAction { protected const ROUTE_PATH = '/tags'; @@ -22,18 +25,26 @@ class ListTagsAction extends AbstractRestAction $this->tagService = $tagService; } - /** - * Process an incoming server request and return a response, optionally delegating - * to the next middleware component to create the response. - * - * - * @throws \InvalidArgumentException - */ public function handle(ServerRequestInterface $request): ResponseInterface { + $query = $request->getQueryParams(); + $withStats = ($query['withStats'] ?? null) === 'true'; + + if (! $withStats) { + return new JsonResponse([ + 'tags' => [ + 'data' => $this->tagService->listTags(), + ], + ]); + } + + $tagsInfo = $this->tagService->tagsInfo(); + $data = map($tagsInfo, fn (TagInfo $info) => (string) $info->tag()); + return new JsonResponse([ 'tags' => [ - 'data' => $this->tagService->listTags(), + 'data' => $data, + 'stats' => $tagsInfo, ], ]); } diff --git a/module/Rest/src/Action/Tag/UpdateTagAction.php b/module/Rest/src/Action/Tag/UpdateTagAction.php index de5eb476..fbf93f50 100644 --- a/module/Rest/src/Action/Tag/UpdateTagAction.php +++ b/module/Rest/src/Action/Tag/UpdateTagAction.php @@ -8,7 +8,7 @@ use Laminas\Diactoros\Response\EmptyResponse; use Psr\Http\Message\ResponseInterface; use Psr\Http\Message\ServerRequestInterface; use Shlinkio\Shlink\Core\Exception\ValidationException; -use Shlinkio\Shlink\Core\Service\Tag\TagServiceInterface; +use Shlinkio\Shlink\Core\Tag\TagServiceInterface; use Shlinkio\Shlink\Rest\Action\AbstractRestAction; class UpdateTagAction extends AbstractRestAction diff --git a/module/Rest/src/Action/Visit/TagVisitsAction.php b/module/Rest/src/Action/Visit/TagVisitsAction.php new file mode 100644 index 00000000..1107ca5c --- /dev/null +++ b/module/Rest/src/Action/Visit/TagVisitsAction.php @@ -0,0 +1,38 @@ +visitsTracker = $visitsTracker; + } + + public function handle(Request $request): Response + { + $tag = $request->getAttribute('tag', ''); + $visits = $this->visitsTracker->visitsForTag($tag, VisitsParams::fromRawData($request->getQueryParams())); + + return new JsonResponse([ + 'visits' => $this->serializePaginator($visits), + ]); + } +} diff --git a/module/Rest/test-api/Action/ListTagsActionTest.php b/module/Rest/test-api/Action/ListTagsActionTest.php new file mode 100644 index 00000000..0690d4f2 --- /dev/null +++ b/module/Rest/test-api/Action/ListTagsActionTest.php @@ -0,0 +1,50 @@ +callApiWithKey(self::METHOD_GET, '/tags', [RequestOptions::QUERY => $query]); + $payload = $this->getJsonResponsePayload($resp); + + $this->assertEquals(['tags' => $expectedTags], $payload); + } + + public function provideQueries(): iterable + { + yield 'stats not requested' => [[], [ + 'data' => ['bar', 'baz', 'foo'], + ]]; + yield 'stats requested' => [['withStats' => 'true'], [ + 'data' => ['bar', 'baz', 'foo'], + 'stats' => [ + [ + 'tag' => 'bar', + 'shortUrlsCount' => 1, + 'visitsCount' => 2, + ], + [ + 'tag' => 'baz', + 'shortUrlsCount' => 0, + 'visitsCount' => 0, + ], + [ + 'tag' => 'foo', + 'shortUrlsCount' => 2, + 'visitsCount' => 5, + ], + ], + ]]; + } +} diff --git a/module/Rest/test-api/Action/TagVisitsActionTest.php b/module/Rest/test-api/Action/TagVisitsActionTest.php new file mode 100644 index 00000000..94e592f6 --- /dev/null +++ b/module/Rest/test-api/Action/TagVisitsActionTest.php @@ -0,0 +1,46 @@ +callApiWithKey(self::METHOD_GET, sprintf('/tags/%s/visits', $tag)); + $payload = $this->getJsonResponsePayload($resp); + + $this->assertArrayHasKey('visits', $payload); + $this->assertArrayHasKey('data', $payload['visits']); + $this->assertCount($expectedVisitsAmount, $payload['visits']['data']); + } + + public function provideTags(): iterable + { + yield 'foo' => ['foo', 5]; + yield 'bar' => ['bar', 2]; + yield 'baz' => ['baz', 0]; + } + + /** @test */ + public function notFoundErrorIsReturnedForInvalidTags(): void + { + $resp = $this->callApiWithKey(self::METHOD_GET, '/tags/invalid_tag/visits'); + $payload = $this->getJsonResponsePayload($resp); + + $this->assertEquals(self::STATUS_NOT_FOUND, $resp->getStatusCode()); + $this->assertEquals(self::STATUS_NOT_FOUND, $payload['status']); + $this->assertEquals('TAG_NOT_FOUND', $payload['type']); + $this->assertEquals('Tag with name "invalid_tag" could not be found', $payload['detail']); + $this->assertEquals('Tag not found', $payload['title']); + } +} diff --git a/module/Rest/test-api/Fixtures/TagsFixture.php b/module/Rest/test-api/Fixtures/TagsFixture.php index 5bd10ca7..5d3333cc 100644 --- a/module/Rest/test-api/Fixtures/TagsFixture.php +++ b/module/Rest/test-api/Fixtures/TagsFixture.php @@ -24,6 +24,7 @@ class TagsFixture extends AbstractFixture implements DependentFixtureInterface $manager->persist($fooTag); $barTag = new Tag('bar'); $manager->persist($barTag); + $manager->persist(new Tag('baz')); /** @var ShortUrl $abcShortUrl */ $abcShortUrl = $this->getReference('abc123_short_url'); diff --git a/module/Rest/test/Action/Tag/CreateTagsActionTest.php b/module/Rest/test/Action/Tag/CreateTagsActionTest.php index 357abc5d..33aa0ba7 100644 --- a/module/Rest/test/Action/Tag/CreateTagsActionTest.php +++ b/module/Rest/test/Action/Tag/CreateTagsActionTest.php @@ -8,7 +8,7 @@ use Doctrine\Common\Collections\ArrayCollection; use Laminas\Diactoros\ServerRequest; use PHPUnit\Framework\TestCase; use Prophecy\Prophecy\ObjectProphecy; -use Shlinkio\Shlink\Core\Service\Tag\TagServiceInterface; +use Shlinkio\Shlink\Core\Tag\TagServiceInterface; use Shlinkio\Shlink\Rest\Action\Tag\CreateTagsAction; class CreateTagsActionTest extends TestCase diff --git a/module/Rest/test/Action/Tag/DeleteTagsActionTest.php b/module/Rest/test/Action/Tag/DeleteTagsActionTest.php index 484bd549..819a608a 100644 --- a/module/Rest/test/Action/Tag/DeleteTagsActionTest.php +++ b/module/Rest/test/Action/Tag/DeleteTagsActionTest.php @@ -7,7 +7,7 @@ namespace ShlinkioTest\Shlink\Rest\Action\Tag; use Laminas\Diactoros\ServerRequest; use PHPUnit\Framework\TestCase; use Prophecy\Prophecy\ObjectProphecy; -use Shlinkio\Shlink\Core\Service\Tag\TagServiceInterface; +use Shlinkio\Shlink\Core\Tag\TagServiceInterface; use Shlinkio\Shlink\Rest\Action\Tag\DeleteTagsAction; class DeleteTagsActionTest extends TestCase diff --git a/module/Rest/test/Action/Tag/ListTagsActionTest.php b/module/Rest/test/Action/Tag/ListTagsActionTest.php index 7e9b061f..461ddd3f 100644 --- a/module/Rest/test/Action/Tag/ListTagsActionTest.php +++ b/module/Rest/test/Action/Tag/ListTagsActionTest.php @@ -4,15 +4,15 @@ declare(strict_types=1); namespace ShlinkioTest\Shlink\Rest\Action\Tag; -use Laminas\Diactoros\ServerRequest; +use Laminas\Diactoros\Response\JsonResponse; +use Laminas\Diactoros\ServerRequestFactory; use PHPUnit\Framework\TestCase; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\Core\Entity\Tag; -use Shlinkio\Shlink\Core\Service\Tag\TagServiceInterface; +use Shlinkio\Shlink\Core\Tag\Model\TagInfo; +use Shlinkio\Shlink\Core\Tag\TagServiceInterface; use Shlinkio\Shlink\Rest\Action\Tag\ListTagsAction; -use function Shlinkio\Shlink\Common\json_decode; - class ListTagsActionTest extends TestCase { private ListTagsAction $action; @@ -24,18 +24,53 @@ class ListTagsActionTest extends TestCase $this->action = new ListTagsAction($this->tagService->reveal()); } - /** @test */ - public function returnsDataFromService(): void + /** + * @test + * @dataProvider provideNoStatsQueries + */ + public function returnsBaseDataWhenStatsAreNotRequested(array $query): void { - $listTags = $this->tagService->listTags()->willReturn([new Tag('foo'), new Tag('bar')]); + $tags = [new Tag('foo'), new Tag('bar')]; + $listTags = $this->tagService->listTags()->willReturn($tags); - $resp = $this->action->handle(new ServerRequest()); + /** @var JsonResponse $resp */ + $resp = $this->action->handle(ServerRequestFactory::fromGlobals()->withQueryParams($query)); + $payload = $resp->getPayload(); + + $this->assertEquals([ + 'tags' => [ + 'data' => $tags, + ], + ], $payload); + $listTags->shouldHaveBeenCalled(); + } + + public 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(new Tag('foo'), 1, 1), + new TagInfo(new Tag('bar'), 3, 10), + ]; + $tagsInfo = $this->tagService->tagsInfo()->willReturn($stats); + + /** @var JsonResponse $resp */ + $resp = $this->action->handle(ServerRequestFactory::fromGlobals()->withQueryParams(['withStats' => 'true'])); + $payload = $resp->getPayload(); $this->assertEquals([ 'tags' => [ 'data' => ['foo', 'bar'], + 'stats' => $stats, ], - ], json_decode((string) $resp->getBody())); - $listTags->shouldHaveBeenCalled(); + ], $payload); + $tagsInfo->shouldHaveBeenCalled(); } } diff --git a/module/Rest/test/Action/Tag/UpdateTagActionTest.php b/module/Rest/test/Action/Tag/UpdateTagActionTest.php index ab09b4ea..11b2c1c4 100644 --- a/module/Rest/test/Action/Tag/UpdateTagActionTest.php +++ b/module/Rest/test/Action/Tag/UpdateTagActionTest.php @@ -9,7 +9,7 @@ use PHPUnit\Framework\TestCase; use Prophecy\Prophecy\ObjectProphecy; use Shlinkio\Shlink\Core\Entity\Tag; use Shlinkio\Shlink\Core\Exception\ValidationException; -use Shlinkio\Shlink\Core\Service\Tag\TagServiceInterface; +use Shlinkio\Shlink\Core\Tag\TagServiceInterface; use Shlinkio\Shlink\Rest\Action\Tag\UpdateTagAction; class UpdateTagActionTest extends TestCase diff --git a/module/Rest/test/Action/Visit/TagVisitsActionTest.php b/module/Rest/test/Action/Visit/TagVisitsActionTest.php new file mode 100644 index 00000000..863bc725 --- /dev/null +++ b/module/Rest/test/Action/Visit/TagVisitsActionTest.php @@ -0,0 +1,41 @@ +visitsTracker = $this->prophesize(VisitsTracker::class); + $this->action = new TagVisitsAction($this->visitsTracker->reveal()); + } + + /** @test */ + public function providingCorrectShortCodeReturnsVisits(): void + { + $tag = 'foo'; + $getVisits = $this->visitsTracker->visitsForTag($tag, Argument::type(VisitsParams::class))->willReturn( + new Paginator(new ArrayAdapter([])), + ); + + $response = $this->action->handle((new ServerRequest())->withAttribute('tag', $tag)); + + $this->assertEquals(200, $response->getStatusCode()); + $getVisits->shouldHaveBeenCalledOnce(); + } +} diff --git a/phpstan.neon b/phpstan.neon index d983a985..e065acef 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -4,3 +4,4 @@ parameters: ignoreErrors: - '#AbstractQuery::setParameters()#' - '#mustRun()#' + - '#AssociationBuilder::setOrderBy#'