From d9ae83a92b9bbdcaf493c61da801bce33bd3bfe3 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 7 May 2020 10:16:20 +0200 Subject: [PATCH 01/33] Updated everything related with dependencies in docker images --- .travis.yml | 2 +- Dockerfile | 11 ++++------- data/infra/php.Dockerfile | 7 ++----- data/infra/swoole.Dockerfile | 9 +++------ docker-compose.yml | 8 ++++---- 5 files changed, 14 insertions(+), 23 deletions(-) diff --git a/.travis.yml b/.travis.yml index a95bf67d..6ab12fa8 100644 --- a/.travis.yml +++ b/.travis.yml @@ -17,7 +17,7 @@ cache: before_install: - 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/Dockerfile b/Dockerfile index 64cd7ebe..3f2c5856 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 @@ -25,15 +25,12 @@ RUN \ # Install swoole and 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 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 shlink 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: From b75922f1d366f78c9f233519e67fffb6e7141979 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 7 May 2020 10:17:34 +0200 Subject: [PATCH 02/33] Updated changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0358e791..14814b99 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this * [#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 From 5f0293bc21491090acfc07dac6a4b77fce06c8ff Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Thu, 7 May 2020 10:45:53 +0200 Subject: [PATCH 03/33] Ensured stable tag is not pushed when building docker image for alpha or beta versions --- docker/build | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docker/build b/docker/build index 5eea7888..f7e4b923 100755 --- a/docker/build +++ b/docker/build @@ -7,7 +7,9 @@ echo "$DOCKER_PASSWORD" | docker login -u "$DOCKER_USERNAME" --password-stdin if [[ ! -z $TRAVIS_TAG ]]; then docker build --build-arg SHLINK_VERSION=${TRAVIS_TAG#?} -t shlinkio/shlink:${TRAVIS_TAG#?} -t shlinkio/shlink:stable . docker push shlinkio/shlink:${TRAVIS_TAG#?} - docker push shlinkio/shlink:stable + + # Push stable tag only if this is not an alpha or beta tag + [[ $TRAVIS_TAG != *"alpha"* && $TRAVIS_TAG != *"beta"* ]] && docker push shlinkio/shlink:stable # If build branch is develop, build latest (on master, when there's no tag, do not build anything) elif [[ "$TRAVIS_BRANCH" == 'develop' ]]; then docker build -t shlinkio/shlink:latest . From 7e0a14493e4b6b4db945adb45a82caf5fbb0b499 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 8 May 2020 10:14:38 +0200 Subject: [PATCH 04/33] Documented updates on the tags endpoint to return more detailed information --- docs/swagger/definitions/TagInfo.json | 17 +++++++++++++++ docs/swagger/paths/v1_tags.json | 21 +++++++++++++++++++ .../Core/src/{Service => }/Tag/TagService.php | 0 .../{Service => }/Tag/TagServiceInterface.php | 0 4 files changed, 38 insertions(+) create mode 100644 docs/swagger/definitions/TagInfo.json rename module/Core/src/{Service => }/Tag/TagService.php (100%) rename module/Core/src/{Service => }/Tag/TagServiceInterface.php (100%) 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/module/Core/src/Service/Tag/TagService.php b/module/Core/src/Tag/TagService.php similarity index 100% rename from module/Core/src/Service/Tag/TagService.php rename to module/Core/src/Tag/TagService.php diff --git a/module/Core/src/Service/Tag/TagServiceInterface.php b/module/Core/src/Tag/TagServiceInterface.php similarity index 100% rename from module/Core/src/Service/Tag/TagServiceInterface.php rename to module/Core/src/Tag/TagServiceInterface.php From 626c92460bcd1d157a70cfb5d6e3bceb18110f47 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 8 May 2020 10:15:33 +0200 Subject: [PATCH 05/33] Enhanced list tags endpoint so that it can also return stats foir every tag --- module/CLI/config/dependencies.config.php | 8 ++-- .../CLI/src/Command/Tag/CreateTagCommand.php | 2 +- .../CLI/src/Command/Tag/DeleteTagsCommand.php | 6 +-- .../CLI/src/Command/Tag/ListTagsCommand.php | 2 +- .../CLI/src/Command/Tag/RenameTagCommand.php | 4 +- .../test/Command/Tag/CreateTagCommandTest.php | 2 +- .../Command/Tag/DeleteTagsCommandTest.php | 4 +- .../test/Command/Tag/ListTagsCommandTest.php | 4 +- .../test/Command/Tag/RenameTagCommandTest.php | 2 +- module/Core/config/dependencies.config.php | 4 +- .../Shlinkio.Shlink.Core.Entity.Tag.php | 6 +++ module/Core/src/Entity/Tag.php | 3 ++ module/Core/src/Repository/TagRepository.php | 23 ++++++++++ .../src/Repository/TagRepositoryInterface.php | 6 +++ module/Core/src/Tag/Model/TagInfo.php | 46 +++++++++++++++++++ module/Core/src/Tag/TagService.php | 15 +++++- module/Core/src/Tag/TagServiceInterface.php | 8 +++- .../Core/test/Service/Tag/TagServiceTest.php | 2 +- module/Rest/config/dependencies.config.php | 8 ++-- .../Rest/src/Action/Tag/CreateTagsAction.php | 2 +- .../Rest/src/Action/Tag/DeleteTagsAction.php | 4 +- module/Rest/src/Action/Tag/ListTagsAction.php | 29 ++++++++---- .../Rest/src/Action/Tag/UpdateTagAction.php | 4 +- .../test/Action/Tag/CreateTagsActionTest.php | 2 +- .../test/Action/Tag/DeleteTagsActionTest.php | 2 +- .../test/Action/Tag/ListTagsActionTest.php | 4 +- .../test/Action/Tag/UpdateTagActionTest.php | 4 +- 27 files changed, 159 insertions(+), 47 deletions(-) create mode 100644 module/Core/src/Tag/Model/TagInfo.php diff --git a/module/CLI/config/dependencies.config.php b/module/CLI/config/dependencies.config.php index 0f2e70a5..1231a9b3 100644 --- a/module/CLI/config/dependencies.config.php +++ b/module/CLI/config/dependencies.config.php @@ -78,10 +78,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 => [\Shlinkio\Shlink\Core\Tag\TagService::class], + Command\Tag\CreateTagCommand::class => [\Shlinkio\Shlink\Core\Tag\TagService::class], + Command\Tag\RenameTagCommand::class => [\Shlinkio\Shlink\Core\Tag\TagService::class], + Command\Tag\DeleteTagsCommand::class => [\Shlinkio\Shlink\Core\Tag\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..f50f835a 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; @@ -16,9 +16,9 @@ class DeleteTagsCommand extends Command { public const NAME = 'tag:delete'; - private TagServiceInterface $tagService; + private \Shlinkio\Shlink\Core\Tag\TagServiceInterface $tagService; - public function __construct(TagServiceInterface $tagService) + public function __construct(\Shlinkio\Shlink\Core\Tag\TagServiceInterface $tagService) { parent::__construct(); $this->tagService = $tagService; diff --git a/module/CLI/src/Command/Tag/ListTagsCommand.php b/module/CLI/src/Command/Tag/ListTagsCommand.php index 0b8f0aa3..5a8389f3 100644 --- a/module/CLI/src/Command/Tag/ListTagsCommand.php +++ b/module/CLI/src/Command/Tag/ListTagsCommand.php @@ -7,7 +7,7 @@ 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\TagServiceInterface; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; diff --git a/module/CLI/src/Command/Tag/RenameTagCommand.php b/module/CLI/src/Command/Tag/RenameTagCommand.php index f30bc757..2a4a1245 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; @@ -20,7 +20,7 @@ class RenameTagCommand extends Command private TagServiceInterface $tagService; - public function __construct(TagServiceInterface $tagService) + public function __construct(\Shlinkio\Shlink\Core\Tag\TagServiceInterface $tagService) { parent::__construct(); $this->tagService = $tagService; 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..1ec75c8f 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; @@ -18,7 +18,7 @@ class DeleteTagsCommandTest extends TestCase public function setUp(): void { - $this->tagService = $this->prophesize(TagServiceInterface::class); + $this->tagService = $this->prophesize(\Shlinkio\Shlink\Core\Tag\TagServiceInterface::class); $command = new DeleteTagsCommand($this->tagService->reveal()); $app = new Application(); diff --git a/module/CLI/test/Command/Tag/ListTagsCommandTest.php b/module/CLI/test/Command/Tag/ListTagsCommandTest.php index f171127c..b3914916 100644 --- a/module/CLI/test/Command/Tag/ListTagsCommandTest.php +++ b/module/CLI/test/Command/Tag/ListTagsCommandTest.php @@ -8,7 +8,7 @@ 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\TagServiceInterface; use Symfony\Component\Console\Application; use Symfony\Component\Console\Tester\CommandTester; @@ -19,7 +19,7 @@ class ListTagsCommandTest extends TestCase public function setUp(): void { - $this->tagService = $this->prophesize(TagServiceInterface::class); + $this->tagService = $this->prophesize(\Shlinkio\Shlink\Core\Tag\TagServiceInterface::class); $command = new ListTagsCommand($this->tagService->reveal()); $app = new Application(); 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..5db524b8 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, @@ -58,7 +58,7 @@ return [ 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.Tag.php b/module/Core/config/entities-mappings/Shlinkio.Shlink.Core.Entity.Tag.php index 214396bd..c3104a9d 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,10 @@ return static function (ClassMetadata $metadata, array $emConfig): void { $builder->createField('name', Types::STRING) ->unique() ->build(); + + $builder->createManyToMany('shortUrls', Entity\ShortUrl::class) + ->setJoinTable(determineTableName('short_urls_in_tags', $emConfig)) + ->addInverseJoinColumn('short_url_id', 'id', true, false, 'CASCADE') + ->addJoinColumn('tag_id', 'id', true, false, 'CASCADE') + ->build(); }; 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/Repository/TagRepository.php b/module/Core/src/Repository/TagRepository.php index 92328630..6f4cbf8c 100644 --- a/module/Core/src/Repository/TagRepository.php +++ b/module/Core/src/Repository/TagRepository.php @@ -6,6 +6,8 @@ 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 +23,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/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/Tag/TagService.php b/module/Core/src/Tag/TagService.php index b95ddf82..7137e885 100644 --- a/module/Core/src/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/Tag/TagServiceInterface.php b/module/Core/src/Tag/TagServiceInterface.php index 16da503c..ed643fc5 100644 --- a/module/Core/src/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/Service/Tag/TagServiceTest.php b/module/Core/test/Service/Tag/TagServiceTest.php index b8c9d59b..bd9c447f 100644 --- a/module/Core/test/Service/Tag/TagServiceTest.php +++ b/module/Core/test/Service/Tag/TagServiceTest.php @@ -13,7 +13,7 @@ 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\TagService; class TagServiceTest extends TestCase { diff --git a/module/Rest/config/dependencies.config.php b/module/Rest/config/dependencies.config.php index bd347897..c7623ff7 100644 --- a/module/Rest/config/dependencies.config.php +++ b/module/Rest/config/dependencies.config.php @@ -65,10 +65,10 @@ return [ 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 => [\Shlinkio\Shlink\Core\Tag\TagService::class], + Action\Tag\DeleteTagsAction::class => [\Shlinkio\Shlink\Core\Tag\TagService::class], + Action\Tag\CreateTagsAction::class => [\Shlinkio\Shlink\Core\Tag\TagService::class], + Action\Tag\UpdateTagAction::class => [\Shlinkio\Shlink\Core\Tag\TagService::class], Middleware\ShortUrl\DropDefaultDomainFromRequestMiddleware::class => ['config.url_shortener.domain.hostname'], Middleware\ShortUrl\DefaultShortCodesLengthMiddleware::class => [ 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..62d13a16 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 @@ -17,7 +17,7 @@ class DeleteTagsAction extends AbstractRestAction private TagServiceInterface $tagService; - public function __construct(TagServiceInterface $tagService) + public function __construct(\Shlinkio\Shlink\Core\Tag\TagServiceInterface $tagService) { $this->tagService = $tagService; } 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..34924f78 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 @@ -16,7 +16,7 @@ class UpdateTagAction extends AbstractRestAction protected const ROUTE_PATH = '/tags'; protected const ROUTE_ALLOWED_METHODS = [self::METHOD_PUT]; - private TagServiceInterface $tagService; + private \Shlinkio\Shlink\Core\Tag\TagServiceInterface $tagService; public function __construct(TagServiceInterface $tagService) { 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..daacb657 100644 --- a/module/Rest/test/Action/Tag/ListTagsActionTest.php +++ b/module/Rest/test/Action/Tag/ListTagsActionTest.php @@ -8,7 +8,7 @@ use Laminas\Diactoros\ServerRequest; 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\TagServiceInterface; use Shlinkio\Shlink\Rest\Action\Tag\ListTagsAction; use function Shlinkio\Shlink\Common\json_decode; @@ -20,7 +20,7 @@ class ListTagsActionTest extends TestCase public function setUp(): void { - $this->tagService = $this->prophesize(TagServiceInterface::class); + $this->tagService = $this->prophesize(\Shlinkio\Shlink\Core\Tag\TagServiceInterface::class); $this->action = new ListTagsAction($this->tagService->reveal()); } diff --git a/module/Rest/test/Action/Tag/UpdateTagActionTest.php b/module/Rest/test/Action/Tag/UpdateTagActionTest.php index ab09b4ea..7d865642 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 @@ -19,7 +19,7 @@ class UpdateTagActionTest extends TestCase public function setUp(): void { - $this->tagService = $this->prophesize(TagServiceInterface::class); + $this->tagService = $this->prophesize(\Shlinkio\Shlink\Core\Tag\TagServiceInterface::class); $this->action = new UpdateTagAction($this->tagService->reveal()); } From 9a78fd1a261441919daffabbe44c9d5f0905fe67 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 8 May 2020 10:25:33 +0200 Subject: [PATCH 06/33] Fixed definition of inversed many to many entity relationship --- .../entities-mappings/Shlinkio.Shlink.Core.Entity.Tag.php | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) 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 c3104a9d..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 @@ -25,9 +25,5 @@ return static function (ClassMetadata $metadata, array $emConfig): void { ->unique() ->build(); - $builder->createManyToMany('shortUrls', Entity\ShortUrl::class) - ->setJoinTable(determineTableName('short_urls_in_tags', $emConfig)) - ->addInverseJoinColumn('short_url_id', 'id', true, false, 'CASCADE') - ->addJoinColumn('tag_id', 'id', true, false, 'CASCADE') - ->build(); + $builder->addInverseManyToMany('shortUrls', Entity\ShortUrl::class, 'tags'); }; From 06c59fe2dddaa05e65e19cb18a1a35b8339e4e5b Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 8 May 2020 10:29:24 +0200 Subject: [PATCH 07/33] Fixed invalid imports after class refactoring --- module/CLI/config/dependencies.config.php | 9 +++++---- module/CLI/src/Command/Tag/DeleteTagsCommand.php | 4 ++-- module/CLI/src/Command/Tag/RenameTagCommand.php | 2 +- module/CLI/test/Command/Tag/DeleteTagsCommandTest.php | 2 +- module/CLI/test/Command/Tag/ListTagsCommandTest.php | 2 +- module/Core/src/Repository/TagRepository.php | 1 + module/Rest/config/dependencies.config.php | 9 +++++---- module/Rest/src/Action/Tag/DeleteTagsAction.php | 2 +- module/Rest/src/Action/Tag/UpdateTagAction.php | 2 +- module/Rest/test/Action/Tag/ListTagsActionTest.php | 2 +- module/Rest/test/Action/Tag/UpdateTagActionTest.php | 2 +- 11 files changed, 20 insertions(+), 17 deletions(-) diff --git a/module/CLI/config/dependencies.config.php b/module/CLI/config/dependencies.config.php index 1231a9b3..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 => [\Shlinkio\Shlink\Core\Tag\TagService::class], - Command\Tag\CreateTagCommand::class => [\Shlinkio\Shlink\Core\Tag\TagService::class], - Command\Tag\RenameTagCommand::class => [\Shlinkio\Shlink\Core\Tag\TagService::class], - Command\Tag\DeleteTagsCommand::class => [\Shlinkio\Shlink\Core\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/DeleteTagsCommand.php b/module/CLI/src/Command/Tag/DeleteTagsCommand.php index f50f835a..2b3eae14 100644 --- a/module/CLI/src/Command/Tag/DeleteTagsCommand.php +++ b/module/CLI/src/Command/Tag/DeleteTagsCommand.php @@ -16,9 +16,9 @@ class DeleteTagsCommand extends Command { public const NAME = 'tag:delete'; - private \Shlinkio\Shlink\Core\Tag\TagServiceInterface $tagService; + private TagServiceInterface $tagService; - public function __construct(\Shlinkio\Shlink\Core\Tag\TagServiceInterface $tagService) + public function __construct(TagServiceInterface $tagService) { parent::__construct(); $this->tagService = $tagService; diff --git a/module/CLI/src/Command/Tag/RenameTagCommand.php b/module/CLI/src/Command/Tag/RenameTagCommand.php index 2a4a1245..fe42a832 100644 --- a/module/CLI/src/Command/Tag/RenameTagCommand.php +++ b/module/CLI/src/Command/Tag/RenameTagCommand.php @@ -20,7 +20,7 @@ class RenameTagCommand extends Command private TagServiceInterface $tagService; - public function __construct(\Shlinkio\Shlink\Core\Tag\TagServiceInterface $tagService) + public function __construct(TagServiceInterface $tagService) { parent::__construct(); $this->tagService = $tagService; diff --git a/module/CLI/test/Command/Tag/DeleteTagsCommandTest.php b/module/CLI/test/Command/Tag/DeleteTagsCommandTest.php index 1ec75c8f..27a95de8 100644 --- a/module/CLI/test/Command/Tag/DeleteTagsCommandTest.php +++ b/module/CLI/test/Command/Tag/DeleteTagsCommandTest.php @@ -18,7 +18,7 @@ class DeleteTagsCommandTest extends TestCase public function setUp(): void { - $this->tagService = $this->prophesize(\Shlinkio\Shlink\Core\Tag\TagServiceInterface::class); + $this->tagService = $this->prophesize(TagServiceInterface::class); $command = new DeleteTagsCommand($this->tagService->reveal()); $app = new Application(); diff --git a/module/CLI/test/Command/Tag/ListTagsCommandTest.php b/module/CLI/test/Command/Tag/ListTagsCommandTest.php index b3914916..4318e906 100644 --- a/module/CLI/test/Command/Tag/ListTagsCommandTest.php +++ b/module/CLI/test/Command/Tag/ListTagsCommandTest.php @@ -19,7 +19,7 @@ class ListTagsCommandTest extends TestCase public function setUp(): void { - $this->tagService = $this->prophesize(\Shlinkio\Shlink\Core\Tag\TagServiceInterface::class); + $this->tagService = $this->prophesize(TagServiceInterface::class); $command = new ListTagsCommand($this->tagService->reveal()); $app = new Application(); diff --git a/module/Core/src/Repository/TagRepository.php b/module/Core/src/Repository/TagRepository.php index 6f4cbf8c..25c05596 100644 --- a/module/Core/src/Repository/TagRepository.php +++ b/module/Core/src/Repository/TagRepository.php @@ -7,6 +7,7 @@ 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 diff --git a/module/Rest/config/dependencies.config.php b/module/Rest/config/dependencies.config.php index c7623ff7..a10fd254 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; @@ -65,10 +66,10 @@ return [ 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 => [\Shlinkio\Shlink\Core\Tag\TagService::class], - Action\Tag\DeleteTagsAction::class => [\Shlinkio\Shlink\Core\Tag\TagService::class], - Action\Tag\CreateTagsAction::class => [\Shlinkio\Shlink\Core\Tag\TagService::class], - Action\Tag\UpdateTagAction::class => [\Shlinkio\Shlink\Core\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/src/Action/Tag/DeleteTagsAction.php b/module/Rest/src/Action/Tag/DeleteTagsAction.php index 62d13a16..f38c443a 100644 --- a/module/Rest/src/Action/Tag/DeleteTagsAction.php +++ b/module/Rest/src/Action/Tag/DeleteTagsAction.php @@ -17,7 +17,7 @@ class DeleteTagsAction extends AbstractRestAction private TagServiceInterface $tagService; - public function __construct(\Shlinkio\Shlink\Core\Tag\TagServiceInterface $tagService) + public function __construct(TagServiceInterface $tagService) { $this->tagService = $tagService; } diff --git a/module/Rest/src/Action/Tag/UpdateTagAction.php b/module/Rest/src/Action/Tag/UpdateTagAction.php index 34924f78..fbf93f50 100644 --- a/module/Rest/src/Action/Tag/UpdateTagAction.php +++ b/module/Rest/src/Action/Tag/UpdateTagAction.php @@ -16,7 +16,7 @@ class UpdateTagAction extends AbstractRestAction protected const ROUTE_PATH = '/tags'; protected const ROUTE_ALLOWED_METHODS = [self::METHOD_PUT]; - private \Shlinkio\Shlink\Core\Tag\TagServiceInterface $tagService; + private TagServiceInterface $tagService; public function __construct(TagServiceInterface $tagService) { diff --git a/module/Rest/test/Action/Tag/ListTagsActionTest.php b/module/Rest/test/Action/Tag/ListTagsActionTest.php index daacb657..813d62f4 100644 --- a/module/Rest/test/Action/Tag/ListTagsActionTest.php +++ b/module/Rest/test/Action/Tag/ListTagsActionTest.php @@ -20,7 +20,7 @@ class ListTagsActionTest extends TestCase public function setUp(): void { - $this->tagService = $this->prophesize(\Shlinkio\Shlink\Core\Tag\TagServiceInterface::class); + $this->tagService = $this->prophesize(TagServiceInterface::class); $this->action = new ListTagsAction($this->tagService->reveal()); } diff --git a/module/Rest/test/Action/Tag/UpdateTagActionTest.php b/module/Rest/test/Action/Tag/UpdateTagActionTest.php index 7d865642..11b2c1c4 100644 --- a/module/Rest/test/Action/Tag/UpdateTagActionTest.php +++ b/module/Rest/test/Action/Tag/UpdateTagActionTest.php @@ -19,7 +19,7 @@ class UpdateTagActionTest extends TestCase public function setUp(): void { - $this->tagService = $this->prophesize(\Shlinkio\Shlink\Core\Tag\TagServiceInterface::class); + $this->tagService = $this->prophesize(TagServiceInterface::class); $this->action = new UpdateTagAction($this->tagService->reveal()); } From bdd14427d921c9969e9b8ffabc51f4339476cedd Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 8 May 2020 11:09:28 +0200 Subject: [PATCH 08/33] Added tests for TagRepository::findTagsWithInfo --- module/Core/src/Repository/TagRepository.php | 2 +- .../test-db/Repository/TagRepositoryTest.php | 57 +++++++++++++++++++ 2 files changed, 58 insertions(+), 1 deletion(-) diff --git a/module/Core/src/Repository/TagRepository.php b/module/Core/src/Repository/TagRepository.php index 25c05596..05b2481c 100644 --- a/module/Core/src/Repository/TagRepository.php +++ b/module/Core/src/Repository/TagRepository.php @@ -35,7 +35,7 @@ class TagRepository extends EntityRepository implements TagRepositoryInterface FROM Shlinkio\Shlink\Core\Entity\Tag t LEFT JOIN t.shortUrls s LEFT JOIN s.visits v - GROUP BY tag + GROUP BY t ORDER BY t.name ASC DQL; $query = $this->getEntityManager()->createQuery($dql); 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(), + ); + } } From 2e269bcacd36994be5756858239e55690d3a82ca Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 8 May 2020 11:14:39 +0200 Subject: [PATCH 09/33] Updated TagServiceTest --- .../Core/test/Service/Tag/TagServiceTest.php | 53 +++++++++---------- 1 file changed, 26 insertions(+), 27 deletions(-) diff --git a/module/Core/test/Service/Tag/TagServiceTest.php b/module/Core/test/Service/Tag/TagServiceTest.php index bd9c447f..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\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); From 91aaffc6db942e5a20b29f5845cdd68bdfafaa73 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 8 May 2020 11:32:06 +0200 Subject: [PATCH 10/33] Updated ListTagsActionTest --- module/Core/src/Tag/Model/TagInfo.php | 10 ---- .../test/Action/Tag/ListTagsActionTest.php | 53 +++++++++++++++---- 2 files changed, 44 insertions(+), 19 deletions(-) diff --git a/module/Core/src/Tag/Model/TagInfo.php b/module/Core/src/Tag/Model/TagInfo.php index dbc51316..0237f062 100644 --- a/module/Core/src/Tag/Model/TagInfo.php +++ b/module/Core/src/Tag/Model/TagInfo.php @@ -25,16 +25,6 @@ final class TagInfo implements JsonSerializable return $this->tag; } - public function shortUrlsCount(): int - { - return $this->shortUrlsCount; - } - - public function visitsCount(): int - { - return $this->visitsCount; - } - public function jsonSerialize(): array { return [ diff --git a/module/Rest/test/Action/Tag/ListTagsActionTest.php b/module/Rest/test/Action/Tag/ListTagsActionTest.php index 813d62f4..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\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(); } } From 00cac4ba720c641f0f817b4fbf4228d9b74c61e8 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 8 May 2020 11:48:31 +0200 Subject: [PATCH 11/33] Created rest test for list tags action --- .../Shlinkio.Shlink.Core.Entity.ShortUrl.php | 1 + .../test-api/Action/ListTagsActionTest.php | 50 +++++++++++++++++++ module/Rest/test-api/Fixtures/TagsFixture.php | 1 + phpstan.neon | 1 + 4 files changed, 53 insertions(+) create mode 100644 module/Rest/test-api/Action/ListTagsActionTest.php 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/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/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/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#' From 252cc7f49d88373cdb01748c32d8901a23ff8151 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 8 May 2020 11:53:26 +0200 Subject: [PATCH 12/33] Updated changelog --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d9fa8531..49fc0a29 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this 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. * [#673](https://github.com/shlinkio/shlink/issues/673) Added new `[GET /visits]` rest endpoint which returns basic visits stats. +* [#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. #### Changed From c336bb19010cfdfeb01d81a8d835d92b946a17de Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 8 May 2020 12:39:02 +0200 Subject: [PATCH 13/33] Updated ListTagsCommand so that it displays extended information --- .../CLI/src/Command/Tag/ListTagsCommand.php | 13 +++++++---- .../test/Command/Tag/ListTagsCommandTest.php | 23 +++++++++++-------- module/Core/src/Tag/Model/TagInfo.php | 10 ++++++++ 3 files changed, 32 insertions(+), 14 deletions(-) diff --git a/module/CLI/src/Command/Tag/ListTagsCommand.php b/module/CLI/src/Command/Tag/ListTagsCommand.php index 5a8389f3..11e22a4f 100644 --- a/module/CLI/src/Command/Tag/ListTagsCommand.php +++ b/module/CLI/src/Command/Tag/ListTagsCommand.php @@ -6,7 +6,7 @@ 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\Tag\Model\TagInfo; use Shlinkio\Shlink\Core\Tag\TagServiceInterface; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputInterface; @@ -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/test/Command/Tag/ListTagsCommandTest.php b/module/CLI/test/Command/Tag/ListTagsCommandTest.php index 4318e906..b6087307 100644 --- a/module/CLI/test/Command/Tag/ListTagsCommandTest.php +++ b/module/CLI/test/Command/Tag/ListTagsCommandTest.php @@ -8,6 +8,7 @@ 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\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/Core/src/Tag/Model/TagInfo.php b/module/Core/src/Tag/Model/TagInfo.php index 0237f062..dbc51316 100644 --- a/module/Core/src/Tag/Model/TagInfo.php +++ b/module/Core/src/Tag/Model/TagInfo.php @@ -25,6 +25,16 @@ final class TagInfo implements JsonSerializable return $this->tag; } + public function shortUrlsCount(): int + { + return $this->shortUrlsCount; + } + + public function visitsCount(): int + { + return $this->visitsCount; + } + public function jsonSerialize(): array { return [ From 7da00fbc8cc2a8115b4f457924463f7a85e8650e Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 8 May 2020 12:58:49 +0200 Subject: [PATCH 14/33] Updated Visit entity so that the address can be optionally obfuscated --- module/Core/src/Entity/Visit.php | 8 +++---- .../Repository/VisitRepositoryTest.php | 8 ++++++- module/Core/test/Entity/VisitTest.php | 24 ++++++++++++++++++- 3 files changed, 34 insertions(+), 6 deletions(-) diff --git a/module/Core/src/Entity/Visit.php b/module/Core/src/Entity/Visit.php index e8cbb119..fb5e3bfb 100644 --- a/module/Core/src/Entity/Visit.php +++ b/module/Core/src/Entity/Visit.php @@ -21,19 +21,19 @@ 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 $obfuscate = 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($obfuscate, $visitor->getRemoteAddress()); } - private function obfuscateAddress(?string $address): ?string + private function processAddress(bool $obfuscate, ?string $address): ?string { // Localhost addresses do not need to be obfuscated - if ($address === null || $address === IpAddress::LOCALHOST) { + if (! $obfuscate || $address === null || $address === IpAddress::LOCALHOST) { return $address; } diff --git a/module/Core/test-db/Repository/VisitRepositoryTest.php b/module/Core/test-db/Repository/VisitRepositoryTest.php index 034b15f9..13fc8581 100644 --- a/module/Core/test-db/Repository/VisitRepositoryTest.php +++ b/module/Core/test-db/Repository/VisitRepositoryTest.php @@ -139,13 +139,19 @@ class VisitRepositoryTest extends DatabaseTestCase $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))); + $visit = new Visit( + $shortUrl, + Visitor::emptyInstance(), + true, + Chronos::parse(sprintf('2016-01-0%s', $i + 1)), + ); $this->getEntityManager()->persist($visit); } 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); diff --git a/module/Core/test/Entity/VisitTest.php b/module/Core/test/Entity/VisitTest.php index 9af71a09..a82e1939 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 addressIsObfuscatedWhenRequested(bool $obfuscate, ?string $address, ?string $expectedAddress): void + { + $visit = new Visit(new ShortUrl(''), new Visitor('Chrome', 'some site', $address), $obfuscate); + + $this->assertEquals($expectedAddress, $visit->getRemoteAddr()); + } + + public function provideAddresses(): iterable + { + yield 'obfuscated null address' => [true, null, null]; + yield 'non-obfuscated null address' => [false, null, null]; + yield 'obfuscated localhost' => [true, IpAddress::LOCALHOST, IpAddress::LOCALHOST]; + yield 'non-obfuscated localhost' => [false, IpAddress::LOCALHOST, IpAddress::LOCALHOST]; + yield 'obfuscated regular address' => [true, '1.2.3.4', '1.2.3.0']; + yield 'non-obfuscated regular address' => [false, '1.2.3.4', '1.2.3.4']; + } } From eac468514ba3e4a6d6909b328170514af630879a Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 8 May 2020 13:10:58 +0200 Subject: [PATCH 15/33] Allow to determine if remote addresses should be obfuscated at configuration level --- config/autoload/url-shortener.global.php | 1 + module/Core/config/dependencies.config.php | 6 +++++- module/Core/src/Service/VisitsTracker.php | 11 ++++++++--- module/Core/test/Service/VisitsTrackerTest.php | 2 +- 4 files changed, 15 insertions(+), 5 deletions(-) diff --git a/config/autoload/url-shortener.global.php b/config/autoload/url-shortener.global.php index 165e0258..8439cc8a 100644 --- a/config/autoload/url-shortener.global.php +++ b/config/autoload/url-shortener.global.php @@ -12,6 +12,7 @@ return [ 'hostname' => '', ], 'validate_url' => false, + 'obfuscate_remote_addr' => true, 'visits_webhooks' => [], 'default_short_codes_length' => DEFAULT_SHORT_CODES_LENGTH, ], diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index 5db524b8..90a4ffaf 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -54,7 +54,11 @@ 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.obfuscate_remote_addr', + ], Service\ShortUrlService::class => ['em', Service\ShortUrl\ShortUrlResolver::class, Util\UrlValidator::class], Visit\VisitLocator::class => ['em'], Visit\VisitsStatsHelper::class => ['em'], diff --git a/module/Core/src/Service/VisitsTracker.php b/module/Core/src/Service/VisitsTracker.php index f477681a..a60513e4 100644 --- a/module/Core/src/Service/VisitsTracker.php +++ b/module/Core/src/Service/VisitsTracker.php @@ -22,11 +22,16 @@ class VisitsTracker implements VisitsTrackerInterface { private ORM\EntityManagerInterface $em; private EventDispatcherInterface $eventDispatcher; + private bool $obfuscateRemoteAddr; - public function __construct(ORM\EntityManagerInterface $em, EventDispatcherInterface $eventDispatcher) - { + public function __construct( + ORM\EntityManagerInterface $em, + EventDispatcherInterface $eventDispatcher, + bool $obfuscateRemoteAddr + ) { $this->em = $em; $this->eventDispatcher = $eventDispatcher; + $this->obfuscateRemoteAddr = $obfuscateRemoteAddr; } /** @@ -34,7 +39,7 @@ class VisitsTracker implements VisitsTrackerInterface */ public function track(ShortUrl $shortUrl, Visitor $visitor): void { - $visit = new Visit($shortUrl, $visitor); + $visit = new Visit($shortUrl, $visitor, $this->obfuscateRemoteAddr); $this->em->persist($visit); $this->em->flush(); diff --git a/module/Core/test/Service/VisitsTrackerTest.php b/module/Core/test/Service/VisitsTrackerTest.php index 9028d2c7..6ae5acf6 100644 --- a/module/Core/test/Service/VisitsTrackerTest.php +++ b/module/Core/test/Service/VisitsTrackerTest.php @@ -37,7 +37,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 */ From ba13d99a71a78b9de839f0f0abed3cc1b19a686f Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 8 May 2020 13:19:40 +0200 Subject: [PATCH 16/33] Allowed remote addr obfuscation to be configured on docker image by using the OBFUSCATE_REMOTE_ADDR env var --- docker/README.md | 8 +++++--- docker/config/shlink_in_docker.local.php | 1 + 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/docker/README.md b/docker/README.md index aa9ae16b..a2283710 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. +* `OBFUSCATE_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 laws. 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 OBFUSCATE_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", + "obfuscate_remote_addr": false } ``` diff --git a/docker/config/shlink_in_docker.local.php b/docker/config/shlink_in_docker.local.php index 5662aaee..d85d0e79 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), + 'obfuscate_remote_addr' => (bool) env('OBFUSCATE_REMOTE_ADDR', true), 'visits_webhooks' => $helper->getVisitsWebhooks(), 'default_short_codes_length' => $helper->getDefaultShortCodesLength(), ], From bfdd6e0c508590b5743be0da36b4395f9b989283 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 8 May 2020 13:21:49 +0200 Subject: [PATCH 17/33] Ensured SimplifiedConfigParser properly handles obfuscate_remote_addr option --- module/Core/src/Config/SimplifiedConfigParser.php | 1 + module/Core/test/Config/SimplifiedConfigParserTest.php | 2 ++ 2 files changed, 3 insertions(+) diff --git a/module/Core/src/Config/SimplifiedConfigParser.php b/module/Core/src/Config/SimplifiedConfigParser.php index fe4a4192..d3c22118 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'], + 'obfuscate_remote_addr' => ['url_shortener', 'obfuscate_remote_addr'], ]; private const SIMPLIFIED_CONFIG_SIDE_EFFECTS = [ 'delete_short_url_threshold' => [ diff --git a/module/Core/test/Config/SimplifiedConfigParserTest.php b/module/Core/test/Config/SimplifiedConfigParserTest.php index 6bc6f1f7..94eb4116 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', + 'obfuscate_remote_addr' => false, ]; $expected = [ 'app_options' => [ @@ -92,6 +93,7 @@ class SimplifiedConfigParserTest extends TestCase 'https://third-party.io/foo', ], 'default_short_codes_length' => 8, + 'obfuscate_remote_addr' => false, ], 'delete_short_urls' => [ From 8f06e4b20fb32085804da8c1c943823d2fb27528 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 8 May 2020 15:43:09 +0200 Subject: [PATCH 18/33] Replaced references to obfuscate by anonymize --- config/autoload/url-shortener.global.php | 2 +- docker/README.md | 6 +++--- docker/config/shlink_in_docker.local.php | 2 +- module/Core/config/dependencies.config.php | 2 +- .../src/Config/SimplifiedConfigParser.php | 2 +- module/Core/src/Entity/Visit.php | 10 +++++----- module/Core/src/Service/VisitsTracker.php | 8 ++++---- .../Config/SimplifiedConfigParserTest.php | 4 ++-- module/Core/test/Entity/VisitTest.php | 16 +++++++-------- .../Core/test/Service/VisitsTrackerTest.php | 20 ------------------- 10 files changed, 26 insertions(+), 46 deletions(-) diff --git a/config/autoload/url-shortener.global.php b/config/autoload/url-shortener.global.php index 8439cc8a..5ad66bea 100644 --- a/config/autoload/url-shortener.global.php +++ b/config/autoload/url-shortener.global.php @@ -12,7 +12,7 @@ return [ 'hostname' => '', ], 'validate_url' => false, - 'obfuscate_remote_addr' => true, + 'anonymize_remote_addr' => true, 'visits_webhooks' => [], 'default_short_codes_length' => DEFAULT_SHORT_CODES_LENGTH, ], diff --git a/docker/README.md b/docker/README.md index a2283710..e17e570e 100644 --- a/docker/README.md +++ b/docker/README.md @@ -173,7 +173,7 @@ This is the complete list of supported env vars: * `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. -* `OBFUSCATE_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 laws. +* `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,7 +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 OBFUSCATE_REMOTE_ADDR=false \ + -e ANONYMIZE_REMOTE_ADDR=false \ shlinkio/shlink:stable ``` @@ -251,7 +251,7 @@ The whole configuration should have this format, but it can be split into multip "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", - "obfuscate_remote_addr": false + "anonymize_remote_addr": false } ``` diff --git a/docker/config/shlink_in_docker.local.php b/docker/config/shlink_in_docker.local.php index d85d0e79..b870ccd7 100644 --- a/docker/config/shlink_in_docker.local.php +++ b/docker/config/shlink_in_docker.local.php @@ -117,7 +117,7 @@ return [ 'hostname' => env('SHORT_DOMAIN_HOST', ''), ], 'validate_url' => (bool) env('VALIDATE_URLS', false), - 'obfuscate_remote_addr' => (bool) env('OBFUSCATE_REMOTE_ADDR', true), + 'anonymize_remote_addr' => (bool) env('ANONYMIZE_REMOTE_ADDR', true), 'visits_webhooks' => $helper->getVisitsWebhooks(), 'default_short_codes_length' => $helper->getDefaultShortCodesLength(), ], diff --git a/module/Core/config/dependencies.config.php b/module/Core/config/dependencies.config.php index 90a4ffaf..debf021f 100644 --- a/module/Core/config/dependencies.config.php +++ b/module/Core/config/dependencies.config.php @@ -57,7 +57,7 @@ return [ Service\VisitsTracker::class => [ 'em', EventDispatcherInterface::class, - 'config.url_shortener.obfuscate_remote_addr', + 'config.url_shortener.anonymize_remote_addr', ], Service\ShortUrlService::class => ['em', Service\ShortUrl\ShortUrlResolver::class, Util\UrlValidator::class], Visit\VisitLocator::class => ['em'], diff --git a/module/Core/src/Config/SimplifiedConfigParser.php b/module/Core/src/Config/SimplifiedConfigParser.php index d3c22118..81f05d14 100644 --- a/module/Core/src/Config/SimplifiedConfigParser.php +++ b/module/Core/src/Config/SimplifiedConfigParser.php @@ -37,7 +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'], - 'obfuscate_remote_addr' => ['url_shortener', 'obfuscate_remote_addr'], + '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/Visit.php b/module/Core/src/Entity/Visit.php index fb5e3bfb..6a4f44e5 100644 --- a/module/Core/src/Entity/Visit.php +++ b/module/Core/src/Entity/Visit.php @@ -21,19 +21,19 @@ class Visit extends AbstractEntity implements JsonSerializable private ShortUrl $shortUrl; private ?VisitLocation $visitLocation = null; - public function __construct(ShortUrl $shortUrl, Visitor $visitor, bool $obfuscate = true, ?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->processAddress($obfuscate, $visitor->getRemoteAddress()); + $this->remoteAddr = $this->processAddress($anonymize, $visitor->getRemoteAddress()); } - private function processAddress(bool $obfuscate, ?string $address): ?string + private function processAddress(bool $anonymize, ?string $address): ?string { - // Localhost addresses do not need to be obfuscated - if (! $obfuscate || $address === null || $address === IpAddress::LOCALHOST) { + // Localhost addresses do not need to be anonymized + if (! $anonymize || $address === null || $address === IpAddress::LOCALHOST) { return $address; } diff --git a/module/Core/src/Service/VisitsTracker.php b/module/Core/src/Service/VisitsTracker.php index a60513e4..39f74cae 100644 --- a/module/Core/src/Service/VisitsTracker.php +++ b/module/Core/src/Service/VisitsTracker.php @@ -22,16 +22,16 @@ class VisitsTracker implements VisitsTrackerInterface { private ORM\EntityManagerInterface $em; private EventDispatcherInterface $eventDispatcher; - private bool $obfuscateRemoteAddr; + private bool $anonymizeRemoteAddr; public function __construct( ORM\EntityManagerInterface $em, EventDispatcherInterface $eventDispatcher, - bool $obfuscateRemoteAddr + bool $anonymizeRemoteAddr ) { $this->em = $em; $this->eventDispatcher = $eventDispatcher; - $this->obfuscateRemoteAddr = $obfuscateRemoteAddr; + $this->anonymizeRemoteAddr = $anonymizeRemoteAddr; } /** @@ -39,7 +39,7 @@ class VisitsTracker implements VisitsTrackerInterface */ public function track(ShortUrl $shortUrl, Visitor $visitor): void { - $visit = new Visit($shortUrl, $visitor, $this->obfuscateRemoteAddr); + $visit = new Visit($shortUrl, $visitor, $this->anonymizeRemoteAddr); $this->em->persist($visit); $this->em->flush(); diff --git a/module/Core/test/Config/SimplifiedConfigParserTest.php b/module/Core/test/Config/SimplifiedConfigParserTest.php index 94eb4116..3700b042 100644 --- a/module/Core/test/Config/SimplifiedConfigParserTest.php +++ b/module/Core/test/Config/SimplifiedConfigParserTest.php @@ -64,7 +64,7 @@ class SimplifiedConfigParserTest extends TestCase 'mercure_public_hub_url' => 'public_url', 'mercure_internal_hub_url' => 'internal_url', 'mercure_jwt_secret' => 'super_secret_value', - 'obfuscate_remote_addr' => false, + 'anonymize_remote_addr' => false, ]; $expected = [ 'app_options' => [ @@ -93,7 +93,7 @@ class SimplifiedConfigParserTest extends TestCase 'https://third-party.io/foo', ], 'default_short_codes_length' => 8, - 'obfuscate_remote_addr' => false, + 'anonymize_remote_addr' => false, ], 'delete_short_urls' => [ diff --git a/module/Core/test/Entity/VisitTest.php b/module/Core/test/Entity/VisitTest.php index a82e1939..73e41f12 100644 --- a/module/Core/test/Entity/VisitTest.php +++ b/module/Core/test/Entity/VisitTest.php @@ -39,20 +39,20 @@ class VisitTest extends TestCase * @test * @dataProvider provideAddresses */ - public function addressIsObfuscatedWhenRequested(bool $obfuscate, ?string $address, ?string $expectedAddress): void + public function addressIsAnonymizedWhenRequested(bool $anonymize, ?string $address, ?string $expectedAddress): void { - $visit = new Visit(new ShortUrl(''), new Visitor('Chrome', 'some site', $address), $obfuscate); + $visit = new Visit(new ShortUrl(''), new Visitor('Chrome', 'some site', $address), $anonymize); $this->assertEquals($expectedAddress, $visit->getRemoteAddr()); } public function provideAddresses(): iterable { - yield 'obfuscated null address' => [true, null, null]; - yield 'non-obfuscated null address' => [false, null, null]; - yield 'obfuscated localhost' => [true, IpAddress::LOCALHOST, IpAddress::LOCALHOST]; - yield 'non-obfuscated localhost' => [false, IpAddress::LOCALHOST, IpAddress::LOCALHOST]; - yield 'obfuscated regular address' => [true, '1.2.3.4', '1.2.3.0']; - yield 'non-obfuscated regular address' => [false, '1.2.3.4', '1.2.3.4']; + 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/Service/VisitsTrackerTest.php b/module/Core/test/Service/VisitsTrackerTest.php index 6ae5acf6..0722352f 100644 --- a/module/Core/test/Service/VisitsTrackerTest.php +++ b/module/Core/test/Service/VisitsTrackerTest.php @@ -6,7 +6,6 @@ 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; @@ -53,25 +52,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 { From f4bf3551f6a13cec6a47a4aeaa77e5762209684f Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 8 May 2020 15:50:16 +0200 Subject: [PATCH 19/33] Updated shlink-installer to a version supporting IP anonymization param --- composer.json | 2 +- config/autoload/installer.global.php | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 5ba58a8e..380cd237 100644 --- a/composer.json +++ b/composer.json @@ -51,7 +51,7 @@ "shlinkio/shlink-common": "dev-master#e659cf9d9b5b3b131419e2f55f2e595f562baafc as 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": "dev-master#50be18de1e505d2609d96c6cc86571b1b1ca7b57 as 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' => [ From e8ab664561c53e8feef7ee95aa650f791fff9c43 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 8 May 2020 15:52:35 +0200 Subject: [PATCH 20/33] Updated changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 49fc0a29..c4b6fd82 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this 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. +* [#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. From 5be882a31b923b6e0e3e60d590bcfaab3b40a775 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 8 May 2020 19:41:21 +0200 Subject: [PATCH 21/33] Improved parameter definition in some private queries in VisitRepository --- .../Core/src/Repository/VisitRepository.php | 69 ++++++++----------- .../Repository/VisitRepositoryInterface.php | 12 ++++ 2 files changed, 42 insertions(+), 39 deletions(-) diff --git a/module/Core/src/Repository/VisitRepository.php b/module/Core/src/Repository/VisitRepository.php index 547493c5..e9d93d10 100644 --- a/module/Core/src/Repository/VisitRepository.php +++ b/module/Core/src/Repository/VisitRepository.php @@ -7,14 +7,11 @@ 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 const PHP_INT_MAX; class VisitRepository extends EntityRepository implements VisitRepositoryInterface @@ -29,7 +26,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 +42,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 +51,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 +86,14 @@ 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); $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. @@ -140,8 +118,7 @@ class VisitRepository extends EntityRepository implements VisitRepositoryInterfa public function countVisitsByShortCode(string $shortCode, ?string $domain = null, ?DateRange $dateRange = null): int { - /** @var QueryBuilder $qb */ - [$qb] = $this->createVisitsByShortCodeQueryBuilder($shortCode, $domain, $dateRange); + $qb = $this->createVisitsByShortCodeQueryBuilder($shortCode, $domain, $dateRange); $qb->select('COUNT(v.id)'); return (int) $qb->getQuery()->getSingleScalarResult(); @@ -151,26 +128,40 @@ class VisitRepository extends EntityRepository implements VisitRepositoryInterfa string $shortCode, ?string $domain, ?DateRange $dateRange - ): array { + ): QueryBuilder { /** @var ShortUrlRepositoryInterface $shortUrlRepo */ $shortUrlRepo = $this->getEntityManager()->getRepository(ShortUrl::class); - $shortUrl = $shortUrlRepo->findOne($shortCode, $domain) ?? -1; + $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', ':shortUrl')) - ->setParameter('shortUrl', $shortUrl); + ->where($qb->expr()->eq('v.shortUrl', $shortUrlId)); // 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); + $qb->andWhere($qb->expr()->gte('v.date', '\'' . $dateRange->getStartDate()->toDateTimeString() . '\'')); } if ($dateRange !== null && $dateRange->getEndDate() !== null) { - $qb->andWhere($qb->expr()->lte('v.date', ':endDate')) - ->setParameter('endDate', $dateRange->getEndDate(), ChronosDateTimeType::CHRONOS_DATETIME); + $qb->andWhere($qb->expr()->lte('v.date', '\'' . $dateRange->getEndDate()->toDateTimeString() . '\'')); } - return [$qb, $shortUrl]; + return $qb; + } + + public function findVisitsByTag( + string $tag, + ?DateRange $dateRange = null, + ?int $limit = null, + ?int $offset = null + ): array { + return []; + } + + public function countVisitsByTag(string $tag, ?DateRange $dateRange = null): int + { + return 0; } } 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; } From baf77b6ffbc9cd859956b5c930c86e71b6f46f68 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 8 May 2020 19:55:05 +0200 Subject: [PATCH 22/33] Implemented methods to get paginated list of visits by tag, reusing methods used for short code filtering --- .../Core/src/Repository/VisitRepository.php | 135 +++++++++++------- 1 file changed, 86 insertions(+), 49 deletions(-) diff --git a/module/Core/src/Repository/VisitRepository.php b/module/Core/src/Repository/VisitRepository.php index e9d93d10..e779a8bb 100644 --- a/module/Core/src/Repository/VisitRepository.php +++ b/module/Core/src/Repository/VisitRepository.php @@ -12,6 +12,8 @@ use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Entity\Visit; use Shlinkio\Shlink\Core\Entity\VisitLocation; +use function array_column; + use const PHP_INT_MAX; class VisitRepository extends EntityRepository implements VisitRepositoryInterface @@ -87,6 +89,90 @@ class VisitRepository extends EntityRepository implements VisitRepositoryInterfa ?int $offset = null ): array { $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); + + // 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', array_column($qb->getQuery()->getArrayResult(), 'id'))); + + // 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 @@ -115,53 +201,4 @@ class VisitRepository extends EntityRepository implements VisitRepositoryInterfa return $query->getResult(); } - - 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 - 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() . '\'')); - } - - return $qb; - } - - public function findVisitsByTag( - string $tag, - ?DateRange $dateRange = null, - ?int $limit = null, - ?int $offset = null - ): array { - return []; - } - - public function countVisitsByTag(string $tag, ?DateRange $dateRange = null): int - { - return 0; - } } From dd4b4277c92f07651b666e91b084236bf4236bb7 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 8 May 2020 20:11:37 +0200 Subject: [PATCH 23/33] Added test for VisitRepository tag methods --- .../Core/src/Repository/VisitRepository.php | 5 +- .../Repository/VisitRepositoryTest.php | 99 +++++++++++++++---- 2 files changed, 85 insertions(+), 19 deletions(-) diff --git a/module/Core/src/Repository/VisitRepository.php b/module/Core/src/Repository/VisitRepository.php index e779a8bb..b3761c9f 100644 --- a/module/Core/src/Repository/VisitRepository.php +++ b/module/Core/src/Repository/VisitRepository.php @@ -149,11 +149,14 @@ class VisitRepository extends EntityRepository implements VisitRepositoryInterfa ->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', array_column($qb->getQuery()->getArrayResult(), 'id'))); + ->where($qb2->expr()->in('v.shortUrl', $shortUrlIds)); // Apply date range filtering $this->applyDatesInline($qb2, $dateRange); diff --git a/module/Core/test-db/Repository/VisitRepositoryTest.php b/module/Core/test-db/Repository/VisitRepositoryTest.php index 13fc8581..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,18 +128,69 @@ 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( @@ -147,17 +201,26 @@ class VisitRepositoryTest extends DatabaseTestCase ); $this->getEntityManager()->persist($visit); } - 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]; + 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]; } } From f0acce1be05e55f9fabe75e23667d2040521e100 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 9 May 2020 09:34:59 +0200 Subject: [PATCH 24/33] Updated to latest common --- composer.json | 2 +- module/Core/src/Entity/Visit.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/composer.json b/composer.json index 380cd237..650e00b5 100644 --- a/composer.json +++ b/composer.json @@ -48,7 +48,7 @@ "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": "dev-master#26109f1e3f1d83e0fc8056d16848ffaca74a8806 as 3.1.0", "shlinkio/shlink-config": "^1.0", "shlinkio/shlink-event-dispatcher": "^1.4", "shlinkio/shlink-installer": "dev-master#50be18de1e505d2609d96c6cc86571b1b1ca7b57 as 5.0.0", diff --git a/module/Core/src/Entity/Visit.php b/module/Core/src/Entity/Visit.php index 6a4f44e5..7e6ed060 100644 --- a/module/Core/src/Entity/Visit.php +++ b/module/Core/src/Entity/Visit.php @@ -38,7 +38,7 @@ class Visit extends AbstractEntity implements JsonSerializable } try { - return (string) IpAddress::fromString($address)->getObfuscatedCopy(); + return (string) IpAddress::fromString($address)->getAnonymizedCopy(); } catch (InvalidArgumentException $e) { return null; } From 3218f8c2835049964ecf60831c6997246f65d6e7 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 9 May 2020 09:53:45 +0200 Subject: [PATCH 25/33] Added Created endpoint to serve visits by tag --- docs/swagger/paths/v2_tags_{tag}_visits.json | 154 ++++++++++++++++++ docs/swagger/swagger.json | 3 + .../Adapter/VisitsForTagPaginatorAdapter.php | 50 ++++++ module/Core/src/Service/VisitsTracker.php | 35 +++- .../src/Service/VisitsTrackerInterface.php | 12 +- module/Rest/config/dependencies.config.php | 2 + module/Rest/config/routes.config.php | 1 + .../Rest/src/Action/Visit/TagVisitsAction.php | 38 +++++ 8 files changed, 283 insertions(+), 12 deletions(-) create mode 100644 docs/swagger/paths/v2_tags_{tag}_visits.json create mode 100644 module/Core/src/Paginator/Adapter/VisitsForTagPaginatorAdapter.php create mode 100644 module/Rest/src/Action/Visit/TagVisitsAction.php 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/Core/src/Paginator/Adapter/VisitsForTagPaginatorAdapter.php b/module/Core/src/Paginator/Adapter/VisitsForTagPaginatorAdapter.php new file mode 100644 index 00000000..d456ad8c --- /dev/null +++ b/module/Core/src/Paginator/Adapter/VisitsForTagPaginatorAdapter.php @@ -0,0 +1,50 @@ +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, + ); + } + + public function count(): 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->countVisitsByTag($this->tag, $this->params->getDateRange()); + } +} diff --git a/module/Core/src/Service/VisitsTracker.php b/module/Core/src/Service/VisitsTracker.php index 39f74cae..e777af76 100644 --- a/module/Core/src/Service/VisitsTracker.php +++ b/module/Core/src/Service/VisitsTracker.php @@ -8,15 +8,19 @@ 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 { @@ -34,9 +38,6 @@ class VisitsTracker implements VisitsTrackerInterface $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, $this->anonymizeRemoteAddr); @@ -48,8 +49,6 @@ class VisitsTracker implements VisitsTrackerInterface } /** - * Returns the visits on certain short code - * * @return Visit[]|Paginator * @throws ShortUrlNotFoundException */ @@ -61,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()) @@ -69,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/Rest/config/dependencies.config.php b/module/Rest/config/dependencies.config.php index a10fd254..258404ef 100644 --- a/module/Rest/config/dependencies.config.php +++ b/module/Rest/config/dependencies.config.php @@ -30,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, @@ -63,6 +64,7 @@ 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], 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/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), + ]); + } +} From e1e3c7f0614e456f76ab75910569386273fcb2b0 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 9 May 2020 10:10:48 +0200 Subject: [PATCH 26/33] Created paginator adapter tests --- ...AbstractCacheableCountPaginatorAdapter.php | 29 ++++++++++ .../Adapter/VisitsForTagPaginatorAdapter.php | 19 +------ .../Adapter/VisitsPaginatorAdapter.php | 19 +------ .../VisitsForTagPaginatorAdapterTest.php | 52 +++++++++++++++++ .../Adapter/VisitsPaginatorAdapterTest.php | 57 +++++++++++++++++++ 5 files changed, 144 insertions(+), 32 deletions(-) create mode 100644 module/Core/src/Paginator/Adapter/AbstractCacheableCountPaginatorAdapter.php create mode 100644 module/Core/test/Paginator/Adapter/VisitsForTagPaginatorAdapterTest.php create mode 100644 module/Core/test/Paginator/Adapter/VisitsPaginatorAdapterTest.php 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 index d456ad8c..e80fbcdd 100644 --- a/module/Core/src/Paginator/Adapter/VisitsForTagPaginatorAdapter.php +++ b/module/Core/src/Paginator/Adapter/VisitsForTagPaginatorAdapter.php @@ -4,18 +4,15 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Paginator\Adapter; -use Laminas\Paginator\Adapter\AdapterInterface; use Shlinkio\Shlink\Core\Model\VisitsParams; use Shlinkio\Shlink\Core\Repository\VisitRepositoryInterface; -class VisitsForTagPaginatorAdapter implements AdapterInterface +class VisitsForTagPaginatorAdapter extends AbstractCacheableCountPaginatorAdapter { private VisitRepositoryInterface $visitRepository; private string $tag; private VisitsParams $params; - private ?int $count = null; - public function __construct(VisitRepositoryInterface $visitRepository, string $tag, VisitsParams $params) { $this->visitRepository = $visitRepository; @@ -33,18 +30,8 @@ class VisitsForTagPaginatorAdapter 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->countVisitsByTag($this->tag, $this->params->getDateRange()); + 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/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(); + } +} From 9b9de8e290a6140c2c8b251e39e27b0e150f11f3 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 9 May 2020 10:14:26 +0200 Subject: [PATCH 27/33] Updated VisitsTrackerTest --- .../Core/test/Service/VisitsTrackerTest.php | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/module/Core/test/Service/VisitsTrackerTest.php b/module/Core/test/Service/VisitsTrackerTest.php index 0722352f..5893b952 100644 --- a/module/Core/test/Service/VisitsTrackerTest.php +++ b/module/Core/test/Service/VisitsTrackerTest.php @@ -12,13 +12,16 @@ 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; @@ -85,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(); + } } From 7f39e6d7681e3c8b67e7d72c3e5e560c2820812c Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 9 May 2020 10:22:07 +0200 Subject: [PATCH 28/33] Created TagVisitsActionTest --- .../test/Action/Visit/TagVisitsActionTest.php | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) create mode 100644 module/Rest/test/Action/Visit/TagVisitsActionTest.php 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(); + } +} From 4d346d1feac64d1e442477640ec7b8e0af902591 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 9 May 2020 10:31:39 +0200 Subject: [PATCH 29/33] Created API test for tags visits endpoint --- .../test-api/Action/TagVisitsActionTest.php | 46 +++++++++++++++++++ 1 file changed, 46 insertions(+) create mode 100644 module/Rest/test-api/Action/TagVisitsActionTest.php 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']); + } +} From 4c5cd88041120b3d556406fe7eaeab3f2943c856 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 9 May 2020 10:36:49 +0200 Subject: [PATCH 30/33] Updated changelog --- CHANGELOG.md | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c4b6fd82..f2491f84 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com), and this project adheres to [Semantic Versioning](https://semver.org). -## [Unreleased] +## 2.2.0 - 2020-05-09 #### Added @@ -19,13 +19,19 @@ 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 From cf605407ad34e3a2b80f1061e9419ec65221946b Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Sat, 9 May 2020 10:56:07 +0200 Subject: [PATCH 31/33] =?UTF-8?q?Used=20definitive=20dependency=20versions?= =?UTF-8?q?=20for=20shlink-common=20and=20shl=C3=B1ink-installer?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- composer.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/composer.json b/composer.json index 650e00b5..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#26109f1e3f1d83e0fc8056d16848ffaca74a8806 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#50be18de1e505d2609d96c6cc86571b1b1ca7b57 as 5.0.0", + "shlinkio/shlink-installer": "^5.0.0", "shlinkio/shlink-ip-geolocation": "^1.4", "symfony/console": "^5.0", "symfony/filesystem": "^5.0", From 1fa989652471f905835c4063505a246cfec12b7e Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 11 May 2020 13:12:55 +0200 Subject: [PATCH 32/33] Fixed error when trying to match creteria on a Short URL with dates --- module/Core/src/Entity/ShortUrl.php | 4 +-- module/Core/test/Entity/ShortUrlTest.php | 35 ++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 2 deletions(-) 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/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, + ]; + } } From 5ef548bc2a4cc652cc2f27ef38ba49802e7579d8 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 11 May 2020 13:19:01 +0200 Subject: [PATCH 33/33] Updated changelog with v2.2.1 --- CHANGELOG.md | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f2491f84..4ede9184 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,29 @@ 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). +## 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