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 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/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/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; } diff --git a/module/Core/src/Paginator/Adapter/AbstractCacheableCountPaginatorAdapter.php b/module/Core/src/Paginator/Adapter/AbstractCacheableCountPaginatorAdapter.php new file mode 100644 index 00000000..cc2a8287 --- /dev/null +++ b/module/Core/src/Paginator/Adapter/AbstractCacheableCountPaginatorAdapter.php @@ -0,0 +1,29 @@ +count !== null) { + return $this->count; + } + + return $this->count = $this->doCount(); + } + + abstract protected function doCount(): int; +} diff --git a/module/Core/src/Paginator/Adapter/VisitsForTagPaginatorAdapter.php b/module/Core/src/Paginator/Adapter/VisitsForTagPaginatorAdapter.php new file mode 100644 index 00000000..e80fbcdd --- /dev/null +++ b/module/Core/src/Paginator/Adapter/VisitsForTagPaginatorAdapter.php @@ -0,0 +1,37 @@ +visitRepository = $visitRepository; + $this->params = $params; + $this->tag = $tag; + } + + public function getItems($offset, $itemCountPerPage): array // phpcs:ignore + { + return $this->visitRepository->findVisitsByTag( + $this->tag, + $this->params->getDateRange(), + $itemCountPerPage, + $offset, + ); + } + + protected function doCount(): int + { + return $this->visitRepository->countVisitsByTag($this->tag, $this->params->getDateRange()); + } +} diff --git a/module/Core/src/Paginator/Adapter/VisitsPaginatorAdapter.php b/module/Core/src/Paginator/Adapter/VisitsPaginatorAdapter.php index 6a42ecbc..404ae309 100644 --- a/module/Core/src/Paginator/Adapter/VisitsPaginatorAdapter.php +++ b/module/Core/src/Paginator/Adapter/VisitsPaginatorAdapter.php @@ -4,19 +4,16 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Paginator\Adapter; -use Laminas\Paginator\Adapter\AdapterInterface; use Shlinkio\Shlink\Core\Model\ShortUrlIdentifier; use Shlinkio\Shlink\Core\Model\VisitsParams; use Shlinkio\Shlink\Core\Repository\VisitRepositoryInterface; -class VisitsPaginatorAdapter implements AdapterInterface +class VisitsPaginatorAdapter extends AbstractCacheableCountPaginatorAdapter { private VisitRepositoryInterface $visitRepository; private ShortUrlIdentifier $identifier; private VisitsParams $params; - private ?int $count = null; - public function __construct( VisitRepositoryInterface $visitRepository, ShortUrlIdentifier $identifier, @@ -38,19 +35,9 @@ class VisitsPaginatorAdapter implements AdapterInterface ); } - public function count(): int + protected function doCount(): int { - // Since a new adapter instance is created every time visits are fetched, it is reasonably safe to internally - // cache the count value. - // The reason it is cached is because the Paginator is actually calling the method twice. - // An inconsistent value could be returned if between the first call and the second one, a new visit is created. - // However, it's almost instant, and then the adapter instance is discarded immediately after. - - if ($this->count !== null) { - return $this->count; - } - - return $this->count = $this->visitRepository->countVisitsByShortCode( + return $this->visitRepository->countVisitsByShortCode( $this->identifier->shortCode(), $this->identifier->domain(), $this->params->getDateRange(), diff --git a/module/Core/src/Repository/VisitRepository.php b/module/Core/src/Repository/VisitRepository.php index 547493c5..b3761c9f 100644 --- a/module/Core/src/Repository/VisitRepository.php +++ b/module/Core/src/Repository/VisitRepository.php @@ -7,13 +7,12 @@ namespace Shlinkio\Shlink\Core\Repository; use Doctrine\ORM\EntityRepository; use Doctrine\ORM\Query\ResultSetMappingBuilder; use Doctrine\ORM\QueryBuilder; -use Shlinkio\Shlink\Common\Doctrine\Type\ChronosDateTimeType; use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\Entity\ShortUrl; use Shlinkio\Shlink\Core\Entity\Visit; use Shlinkio\Shlink\Core\Entity\VisitLocation; -use function preg_replace; +use function array_column; use const PHP_INT_MAX; @@ -29,7 +28,7 @@ class VisitRepository extends EntityRepository implements VisitRepositoryInterfa ->from(Visit::class, 'v') ->where($qb->expr()->isNull('v.visitLocation')); - return $this->findVisitsForQuery($qb, $blockSize); + return $this->visitsIterableForQuery($qb, $blockSize); } /** @@ -45,7 +44,7 @@ class VisitRepository extends EntityRepository implements VisitRepositoryInterfa ->andWhere($qb->expr()->eq('vl.isEmpty', ':isEmpty')) ->setParameter('isEmpty', true); - return $this->findVisitsForQuery($qb, $blockSize); + return $this->visitsIterableForQuery($qb, $blockSize); } public function findAllVisits(int $blockSize = self::DEFAULT_BLOCK_SIZE): iterable @@ -54,10 +53,10 @@ class VisitRepository extends EntityRepository implements VisitRepositoryInterfa $qb->select('v') ->from(Visit::class, 'v'); - return $this->findVisitsForQuery($qb, $blockSize); + return $this->visitsIterableForQuery($qb, $blockSize); } - private function findVisitsForQuery(QueryBuilder $qb, int $blockSize): iterable + private function visitsIterableForQuery(QueryBuilder $qb, int $blockSize): iterable { $originalQueryBuilder = $qb->setMaxResults($blockSize) ->orderBy('v.id', 'ASC'); @@ -89,33 +88,101 @@ class VisitRepository extends EntityRepository implements VisitRepositoryInterfa ?int $limit = null, ?int $offset = null ): array { - /** - * @var QueryBuilder $qb - * @var ShortUrl|int $shortUrl - */ - [$qb, $shortUrl] = $this->createVisitsByShortCodeQueryBuilder($shortCode, $domain, $dateRange); + $qb = $this->createVisitsByShortCodeQueryBuilder($shortCode, $domain, $dateRange); + return $this->resolveVisitsWithNativeQuery($qb, $limit, $offset); + } + + public function countVisitsByShortCode(string $shortCode, ?string $domain = null, ?DateRange $dateRange = null): int + { + $qb = $this->createVisitsByShortCodeQueryBuilder($shortCode, $domain, $dateRange); + $qb->select('COUNT(v.id)'); + + return (int) $qb->getQuery()->getSingleScalarResult(); + } + + private function createVisitsByShortCodeQueryBuilder( + string $shortCode, + ?string $domain, + ?DateRange $dateRange + ): QueryBuilder { + /** @var ShortUrlRepositoryInterface $shortUrlRepo */ + $shortUrlRepo = $this->getEntityManager()->getRepository(ShortUrl::class); + $shortUrl = $shortUrlRepo->findOne($shortCode, $domain); + $shortUrlId = $shortUrl !== null ? $shortUrl->getId() : -1; + + // Parameters in this query need to be part of the query itself, as we need to use it a sub-query later + // Since they are not strictly provided by the caller, it's reasonably safe + $qb = $this->getEntityManager()->createQueryBuilder(); + $qb->from(Visit::class, 'v') + ->where($qb->expr()->eq('v.shortUrl', $shortUrlId)); + + // Apply date range filtering + $this->applyDatesInline($qb, $dateRange); + + return $qb; + } + + public function findVisitsByTag( + string $tag, + ?DateRange $dateRange = null, + ?int $limit = null, + ?int $offset = null + ): array { + $qb = $this->createVisitsByTagQueryBuilder($tag, $dateRange); + return $this->resolveVisitsWithNativeQuery($qb, $limit, $offset); + } + + public function countVisitsByTag(string $tag, ?DateRange $dateRange = null): int + { + $qb = $this->createVisitsByTagQueryBuilder($tag, $dateRange); + $qb->select('COUNT(v.id)'); + + return (int) $qb->getQuery()->getSingleScalarResult(); + } + + private function createVisitsByTagQueryBuilder(string $tag, ?DateRange $dateRange = null): QueryBuilder + { + $qb = $this->getEntityManager()->createQueryBuilder(); + $qb->select('s.id') + ->from(ShortUrl::class, 's') + ->join('s.tags', 't') + ->where($qb->expr()->eq('t.name', ':tag')) + ->setParameter('tag', $tag); + + $shortUrlIds = array_column($qb->getQuery()->getArrayResult(), 'id'); + $shortUrlIds[] = '-1'; // Add an invalid ID, in case the list is empty + + // Parameters in this query need to be part of the query itself, as we need to use it a sub-query later + // Since they are not strictly provided by the caller, it's reasonably safe + $qb2 = $this->getEntityManager()->createQueryBuilder(); + $qb2->from(Visit::class, 'v') + ->where($qb2->expr()->in('v.shortUrl', $shortUrlIds)); + + // Apply date range filtering + $this->applyDatesInline($qb2, $dateRange); + + return $qb2; + } + + private function applyDatesInline(QueryBuilder $qb, ?DateRange $dateRange): void + { + if ($dateRange !== null && $dateRange->getStartDate() !== null) { + $qb->andWhere($qb->expr()->gte('v.date', '\'' . $dateRange->getStartDate()->toDateTimeString() . '\'')); + } + if ($dateRange !== null && $dateRange->getEndDate() !== null) { + $qb->andWhere($qb->expr()->lte('v.date', '\'' . $dateRange->getEndDate()->toDateTimeString() . '\'')); + } + } + + private function resolveVisitsWithNativeQuery(QueryBuilder $qb, ?int $limit, ?int $offset): array + { $qb->select('v.id') ->orderBy('v.id', 'DESC') // Falling back to values that will behave as no limit/offset, but will workaround MS SQL not allowing // order on sub-queries without offset ->setMaxResults($limit ?? PHP_INT_MAX) ->setFirstResult($offset ?? 0); - - // FIXME Crappy way to resolve the params into the query. Best option would be to inject the sub-query with - // placeholders and then pass params to the main query - $shortUrlId = $shortUrl instanceof ShortUrl ? $shortUrl->getId() : $shortUrl; - $subQuery = preg_replace('/\?/', $shortUrlId, $qb->getQuery()->getSQL(), 1); - if ($dateRange !== null && $dateRange->getStartDate() !== null) { - $subQuery = preg_replace( - '/\?/', - '\'' . $dateRange->getStartDate()->toDateTimeString() . '\'', - $subQuery, - 1, - ); - } - if ($dateRange !== null && $dateRange->getEndDate() !== null) { - $subQuery = preg_replace('/\?/', '\'' . $dateRange->getEndDate()->toDateTimeString() . '\'', $subQuery, 1); - } + $subQuery = $qb->getQuery()->getSQL(); // A native query builder needs to be used here because DQL and ORM query builders do not accept // sub-queries at "from" and "join" level. @@ -137,40 +204,4 @@ class VisitRepository extends EntityRepository implements VisitRepositoryInterfa return $query->getResult(); } - - public function countVisitsByShortCode(string $shortCode, ?string $domain = null, ?DateRange $dateRange = null): int - { - /** @var QueryBuilder $qb */ - [$qb] = $this->createVisitsByShortCodeQueryBuilder($shortCode, $domain, $dateRange); - $qb->select('COUNT(v.id)'); - - return (int) $qb->getQuery()->getSingleScalarResult(); - } - - private function createVisitsByShortCodeQueryBuilder( - string $shortCode, - ?string $domain, - ?DateRange $dateRange - ): array { - /** @var ShortUrlRepositoryInterface $shortUrlRepo */ - $shortUrlRepo = $this->getEntityManager()->getRepository(ShortUrl::class); - $shortUrl = $shortUrlRepo->findOne($shortCode, $domain) ?? -1; - - $qb = $this->getEntityManager()->createQueryBuilder(); - $qb->from(Visit::class, 'v') - ->where($qb->expr()->eq('v.shortUrl', ':shortUrl')) - ->setParameter('shortUrl', $shortUrl); - - // Apply date range filtering - if ($dateRange !== null && $dateRange->getStartDate() !== null) { - $qb->andWhere($qb->expr()->gte('v.date', ':startDate')) - ->setParameter('startDate', $dateRange->getStartDate(), ChronosDateTimeType::CHRONOS_DATETIME); - } - if ($dateRange !== null && $dateRange->getEndDate() !== null) { - $qb->andWhere($qb->expr()->lte('v.date', ':endDate')) - ->setParameter('endDate', $dateRange->getEndDate(), ChronosDateTimeType::CHRONOS_DATETIME); - } - - return [$qb, $shortUrl]; - } } diff --git a/module/Core/src/Repository/VisitRepositoryInterface.php b/module/Core/src/Repository/VisitRepositoryInterface.php index f9cbc8d9..5a540171 100644 --- a/module/Core/src/Repository/VisitRepositoryInterface.php +++ b/module/Core/src/Repository/VisitRepositoryInterface.php @@ -43,4 +43,16 @@ interface VisitRepositoryInterface extends ObjectRepository ?string $domain = null, ?DateRange $dateRange = null ): int; + + /** + * @return Visit[] + */ + public function findVisitsByTag( + string $tag, + ?DateRange $dateRange = null, + ?int $limit = null, + ?int $offset = null + ): array; + + public function countVisitsByTag(string $tag, ?DateRange $dateRange = null): int; } diff --git a/module/Core/src/Service/VisitsTracker.php b/module/Core/src/Service/VisitsTracker.php index 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/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]; } } diff --git a/module/Core/test/Paginator/Adapter/VisitsForTagPaginatorAdapterTest.php b/module/Core/test/Paginator/Adapter/VisitsForTagPaginatorAdapterTest.php new file mode 100644 index 00000000..e4418c5b --- /dev/null +++ b/module/Core/test/Paginator/Adapter/VisitsForTagPaginatorAdapterTest.php @@ -0,0 +1,52 @@ +repo = $this->prophesize(VisitRepositoryInterface::class); + $this->adapter = new VisitsForTagPaginatorAdapter($this->repo->reveal(), 'foo', VisitsParams::fromRawData([])); + } + + /** @test */ + public function repoIsCalledEveryTimeItemsAreFetched(): void + { + $count = 3; + $limit = 1; + $offset = 5; + $findVisits = $this->repo->findVisitsByTag('foo', new DateRange(), $limit, $offset)->willReturn([]); + + for ($i = 0; $i < $count; $i++) { + $this->adapter->getItems($offset, $limit); + } + + $findVisits->shouldHaveBeenCalledTimes($count); + } + + /** @test */ + public function repoIsCalledOnlyOnceForCount(): void + { + $count = 3; + $countVisits = $this->repo->countVisitsByTag('foo', new DateRange())->willReturn(3); + + for ($i = 0; $i < $count; $i++) { + $this->adapter->count(); + } + + $countVisits->shouldHaveBeenCalledOnce(); + } +} diff --git a/module/Core/test/Paginator/Adapter/VisitsPaginatorAdapterTest.php b/module/Core/test/Paginator/Adapter/VisitsPaginatorAdapterTest.php new file mode 100644 index 00000000..744582b7 --- /dev/null +++ b/module/Core/test/Paginator/Adapter/VisitsPaginatorAdapterTest.php @@ -0,0 +1,57 @@ +repo = $this->prophesize(VisitRepositoryInterface::class); + $this->adapter = new VisitsPaginatorAdapter( + $this->repo->reveal(), + new ShortUrlIdentifier(''), + VisitsParams::fromRawData([]), + ); + } + + /** @test */ + public function repoIsCalledEveryTimeItemsAreFetched(): void + { + $count = 3; + $limit = 1; + $offset = 5; + $findVisits = $this->repo->findVisitsByShortCode('', null, new DateRange(), $limit, $offset)->willReturn([]); + + for ($i = 0; $i < $count; $i++) { + $this->adapter->getItems($offset, $limit); + } + + $findVisits->shouldHaveBeenCalledTimes($count); + } + + /** @test */ + public function repoIsCalledOnlyOnceForCount(): void + { + $count = 3; + $countVisits = $this->repo->countVisitsByShortCode('', null, new DateRange())->willReturn(3); + + for ($i = 0; $i < $count; $i++) { + $this->adapter->count(); + } + + $countVisits->shouldHaveBeenCalledOnce(); + } +} diff --git a/module/Core/test/Service/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(); + } } 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), + ]); + } +} diff --git a/module/Rest/test-api/Action/TagVisitsActionTest.php b/module/Rest/test-api/Action/TagVisitsActionTest.php new file mode 100644 index 00000000..94e592f6 --- /dev/null +++ b/module/Rest/test-api/Action/TagVisitsActionTest.php @@ -0,0 +1,46 @@ +callApiWithKey(self::METHOD_GET, sprintf('/tags/%s/visits', $tag)); + $payload = $this->getJsonResponsePayload($resp); + + $this->assertArrayHasKey('visits', $payload); + $this->assertArrayHasKey('data', $payload['visits']); + $this->assertCount($expectedVisitsAmount, $payload['visits']['data']); + } + + public function provideTags(): iterable + { + yield 'foo' => ['foo', 5]; + yield 'bar' => ['bar', 2]; + yield 'baz' => ['baz', 0]; + } + + /** @test */ + public function notFoundErrorIsReturnedForInvalidTags(): void + { + $resp = $this->callApiWithKey(self::METHOD_GET, '/tags/invalid_tag/visits'); + $payload = $this->getJsonResponsePayload($resp); + + $this->assertEquals(self::STATUS_NOT_FOUND, $resp->getStatusCode()); + $this->assertEquals(self::STATUS_NOT_FOUND, $payload['status']); + $this->assertEquals('TAG_NOT_FOUND', $payload['type']); + $this->assertEquals('Tag with name "invalid_tag" could not be found', $payload['detail']); + $this->assertEquals('Tag not found', $payload['title']); + } +} diff --git a/module/Rest/test/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(); + } +}