From 961178fd82d3c91a06f91bafd8bf998b5e2fd0fd Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 2 Jan 2023 19:28:32 +0100 Subject: [PATCH 1/6] Added amount of bots, non-bots and total visits to the list of tags with stats --- .../CLI/src/Command/Tag/ListTagsCommand.php | 2 +- module/Core/functions/functions.php | 12 +++ module/Core/src/Tag/Model/OrderableField.php | 30 +++++++ module/Core/src/Tag/Model/TagInfo.php | 19 ++++- .../Core/src/Tag/Repository/TagRepository.php | 19 +++-- module/Core/src/Tag/TagService.php | 2 +- .../Tag/Repository/TagRepositoryTest.php | 83 ++++++++++--------- .../Rest/src/Action/Tag/TagsStatsAction.php | 2 +- module/Rest/test-api/Action/TagsStatsTest.php | 45 ++++++++++ 9 files changed, 159 insertions(+), 55 deletions(-) create mode 100644 module/Core/src/Tag/Model/OrderableField.php diff --git a/module/CLI/src/Command/Tag/ListTagsCommand.php b/module/CLI/src/Command/Tag/ListTagsCommand.php index cd820169..02116d79 100644 --- a/module/CLI/src/Command/Tag/ListTagsCommand.php +++ b/module/CLI/src/Command/Tag/ListTagsCommand.php @@ -46,7 +46,7 @@ class ListTagsCommand extends Command return map( $tags, - static fn (TagInfo $tagInfo) => [$tagInfo->tag, $tagInfo->shortUrlsCount, $tagInfo->visitsCount], + static fn (TagInfo $tagInfo) => [$tagInfo->tag, $tagInfo->shortUrlsCount, $tagInfo->visitsSummary->total], ); } } diff --git a/module/Core/functions/functions.php b/module/Core/functions/functions.php index e7dff2ad..9d0b8d68 100644 --- a/module/Core/functions/functions.php +++ b/module/Core/functions/functions.php @@ -10,6 +10,7 @@ use DateTimeInterface; use Doctrine\ORM\Mapping\Builder\FieldBuilder; use Jaybizzle\CrawlerDetect\CrawlerDetect; use Laminas\Filter\Word\CamelCaseToSeparator; +use Laminas\Filter\Word\CamelCaseToUnderscore; use Laminas\InputFilter\InputFilter; use PUGX\Shortid\Factory as ShortIdFactory; use Shlinkio\Shlink\Common\Util\DateRange; @@ -21,6 +22,7 @@ use function print_r; use function Shlinkio\Shlink\Common\buildDateRange; use function sprintf; use function str_repeat; +use function strtolower; use function ucfirst; function generateRandomShortCode(int $length): string @@ -143,6 +145,16 @@ function camelCaseToHumanFriendly(string $value): string return ucfirst($filter->filter($value)); } +function camelCaseToSnakeCase(string $value): string +{ + static $filter; + if ($filter === null) { + $filter = new CamelCaseToUnderscore(); + } + + return strtolower($filter->filter($value)); +} + function toProblemDetailsType(string $errorCode): string { return sprintf('https://shlink.io/api/error/%s', $errorCode); diff --git a/module/Core/src/Tag/Model/OrderableField.php b/module/Core/src/Tag/Model/OrderableField.php new file mode 100644 index 00000000..cb398ebb --- /dev/null +++ b/module/Core/src/Tag/Model/OrderableField.php @@ -0,0 +1,30 @@ +value || $field === self::VISITS_COUNT->value; + } + + public static function toSnakeCaseValidField(?string $field): string + { + return camelCaseToSnakeCase($field === self::SHORT_URLS_COUNT->value ? $field : self::VISITS_COUNT->value); + } +} diff --git a/module/Core/src/Tag/Model/TagInfo.php b/module/Core/src/Tag/Model/TagInfo.php index 5e71ea5b..adf5d4b1 100644 --- a/module/Core/src/Tag/Model/TagInfo.php +++ b/module/Core/src/Tag/Model/TagInfo.php @@ -5,19 +5,29 @@ declare(strict_types=1); namespace Shlinkio\Shlink\Core\Tag\Model; use JsonSerializable; +use Shlinkio\Shlink\Core\Visit\Model\VisitsSummary; final class TagInfo implements JsonSerializable { + public readonly VisitsSummary $visitsSummary; + public function __construct( public readonly string $tag, public readonly int $shortUrlsCount, - public readonly int $visitsCount, + int $visitsCount, + ?int $nonBotVisitsCount = null, ) { + $this->visitsSummary = VisitsSummary::fromTotalAndNonBots($visitsCount, $nonBotVisitsCount ?? $visitsCount); } public static function fromRawData(array $data): self { - return new self($data['tag'], (int) $data['shortUrlsCount'], (int) $data['visitsCount']); + return new self( + $data['tag'], + (int) $data['shortUrlsCount'], + (int) $data['visitsCount'], + isset($data['nonBotVisitsCount']) ? (int) $data['nonBotVisitsCount'] : null, + ); } public function jsonSerialize(): array @@ -25,7 +35,10 @@ final class TagInfo implements JsonSerializable return [ 'tag' => $this->tag, 'shortUrlsCount' => $this->shortUrlsCount, - 'visitsCount' => $this->visitsCount, + 'visitsSummary' => $this->visitsSummary, + + // Deprecated + 'visitsCount' => $this->visitsSummary->total, ]; } } diff --git a/module/Core/src/Tag/Repository/TagRepository.php b/module/Core/src/Tag/Repository/TagRepository.php index 88e817ad..feaaa7d5 100644 --- a/module/Core/src/Tag/Repository/TagRepository.php +++ b/module/Core/src/Tag/Repository/TagRepository.php @@ -8,6 +8,7 @@ use Doctrine\ORM\Query\ResultSetMappingBuilder; use Happyr\DoctrineSpecification\Repository\EntitySpecificationRepository; use Happyr\DoctrineSpecification\Spec; use Shlinkio\Shlink\Core\Tag\Entity\Tag; +use Shlinkio\Shlink\Core\Tag\Model\OrderableField; use Shlinkio\Shlink\Core\Tag\Model\TagInfo; use Shlinkio\Shlink\Core\Tag\Model\TagsListFiltering; use Shlinkio\Shlink\Core\Tag\Spec\CountTagsWithName; @@ -16,7 +17,6 @@ use Shlinkio\Shlink\Rest\ApiKey\Spec\WithApiKeySpecsEnsuringJoin; use Shlinkio\Shlink\Rest\ApiKey\Spec\WithInlinedApiKeySpecsEnsuringJoin; use Shlinkio\Shlink\Rest\Entity\ApiKey; -use function Functional\contains; use function Functional\map; use const PHP_INT_MAX; @@ -43,7 +43,7 @@ class TagRepository extends EntitySpecificationRepository implements TagReposito { $orderField = $filtering?->orderBy?->field; $orderDir = $filtering?->orderBy?->direction; - $orderMainQuery = contains(['shortUrlsCount', 'visitsCount'], $orderField); + $orderMainQuery = $orderField !== null && OrderableField::isAggregateField($orderField); $conn = $this->getEntityManager()->getConnection(); $subQb = $this->createQueryBuilder('t'); @@ -72,12 +72,17 @@ class TagRepository extends EntitySpecificationRepository implements TagReposito 't.id_0 AS id', 't.name_1 AS name', 'COUNT(DISTINCT s.id) AS short_urls_count', - 'COUNT(DISTINCT v.id) AS visits_count', + 'COUNT(DISTINCT v.id) AS visits_count', // Native queries require snake_case for cross-db compatibility + 'COUNT(DISTINCT v2.id) AS non_bot_visits_count', ) ->from('(' . $subQb->getQuery()->getSQL() . ')', 't') // @phpstan-ignore-line ->leftJoin('t', 'short_urls_in_tags', 'st', $nativeQb->expr()->eq('t.id_0', 'st.tag_id')) ->leftJoin('st', 'short_urls', 's', $nativeQb->expr()->eq('s.id', 'st.short_url_id')) - ->leftJoin('st', 'visits', 'v', $nativeQb->expr()->eq('s.id', 'v.short_url_id')) + ->leftJoin('st', 'visits', 'v', $nativeQb->expr()->eq('st.short_url_id', 'v.short_url_id')) + ->leftJoin('st', 'visits', 'v2', $nativeQb->expr()->and( // @phpstan-ignore-line + $nativeQb->expr()->eq('st.short_url_id', 'v2.short_url_id'), + $nativeQb->expr()->eq('v2.potential_bot', $conn->quote('0')), + )) ->groupBy('t.id_0', 't.name_1'); // Apply API key role conditions to the native query too, as they will affect the amounts on the aggregates @@ -92,10 +97,7 @@ class TagRepository extends EntitySpecificationRepository implements TagReposito if ($orderMainQuery) { $nativeQb - ->orderBy( - $orderField === 'shortUrlsCount' ? 'short_urls_count' : 'visits_count', - $orderDir ?? 'ASC', - ) + ->orderBy(OrderableField::toSnakeCaseValidField($orderField), $orderDir ?? 'ASC') ->setMaxResults($filtering?->limit ?? PHP_INT_MAX) ->setFirstResult($filtering?->offset ?? 0); } @@ -107,6 +109,7 @@ class TagRepository extends EntitySpecificationRepository implements TagReposito $rsm->addScalarResult('name', 'tag'); $rsm->addScalarResult('short_urls_count', 'shortUrlsCount'); $rsm->addScalarResult('visits_count', 'visitsCount'); + $rsm->addScalarResult('non_bot_visits_count', 'nonBotVisitsCount'); return map( $this->getEntityManager()->createNativeQuery($nativeQb->getSQL(), $rsm)->getResult(), diff --git a/module/Core/src/Tag/TagService.php b/module/Core/src/Tag/TagService.php index 66e031d3..d50ced75 100644 --- a/module/Core/src/Tag/TagService.php +++ b/module/Core/src/Tag/TagService.php @@ -22,7 +22,7 @@ use Shlinkio\Shlink\Rest\Entity\ApiKey; class TagService implements TagServiceInterface { - public function __construct(private ORM\EntityManagerInterface $em) + public function __construct(private readonly ORM\EntityManagerInterface $em) { } diff --git a/module/Core/test-db/Tag/Repository/TagRepositoryTest.php b/module/Core/test-db/Tag/Repository/TagRepositoryTest.php index ce0efff9..fe030ca0 100644 --- a/module/Core/test-db/Tag/Repository/TagRepositoryTest.php +++ b/module/Core/test-db/Tag/Repository/TagRepositoryTest.php @@ -73,7 +73,7 @@ class TagRepositoryTest extends DatabaseTestCase [$firstUrlTags] = array_chunk($names, 3); $secondUrlTags = [$names[0]]; - $metaWithTags = fn (array $tags, ?ApiKey $apiKey) => ShortUrlCreation::fromRawData( + $metaWithTags = static fn (array $tags, ?ApiKey $apiKey) => ShortUrlCreation::fromRawData( ['longUrl' => '', 'tags' => $tags, 'apiKey' => $apiKey], ); @@ -81,7 +81,7 @@ class TagRepositoryTest extends DatabaseTestCase $this->getEntityManager()->persist($shortUrl); $this->getEntityManager()->persist(Visit::forValidShortUrl($shortUrl, Visitor::emptyInstance())); $this->getEntityManager()->persist(Visit::forValidShortUrl($shortUrl, Visitor::emptyInstance())); - $this->getEntityManager()->persist(Visit::forValidShortUrl($shortUrl, Visitor::emptyInstance())); + $this->getEntityManager()->persist(Visit::forValidShortUrl($shortUrl, Visitor::botInstance())); $shortUrl2 = ShortUrl::create($metaWithTags($secondUrlTags, null), $this->relationResolver); $this->getEntityManager()->persist($shortUrl2); @@ -100,9 +100,10 @@ class TagRepositoryTest extends DatabaseTestCase $result = $this->repo->findTagsWithInfo($filtering); self::assertCount(count($expectedList), $result); - foreach ($expectedList as $index => [$tag, $shortUrlsCount, $visitsCount]) { + foreach ($expectedList as $index => [$tag, $shortUrlsCount, $visitsCount, $nonBotVisitsCount]) { self::assertEquals($shortUrlsCount, $result[$index]->shortUrlsCount); - self::assertEquals($visitsCount, $result[$index]->visitsCount); + self::assertEquals($visitsCount, $result[$index]->visitsSummary->total); + self::assertEquals($nonBotVisitsCount, $result[$index]->visitsSummary->nonBots); self::assertEquals($tag, $result[$index]->tag); } } @@ -110,95 +111,95 @@ class TagRepositoryTest extends DatabaseTestCase public function provideFilters(): iterable { $defaultList = [ - ['another', 0, 0], - ['bar', 3, 3], - ['baz', 1, 3], - ['foo', 2, 4], + ['another', 0, 0, 0], + ['bar', 3, 3, 2], + ['baz', 1, 3, 2], + ['foo', 2, 4, 3], ]; yield 'no filter' => [null, $defaultList]; yield 'empty filter' => [new TagsListFiltering(), $defaultList]; yield 'limit' => [new TagsListFiltering(2), [ - ['another', 0, 0], - ['bar', 3, 3], + ['another', 0, 0, 0], + ['bar', 3, 3, 2], ]]; yield 'offset' => [new TagsListFiltering(null, 3), [ - ['foo', 2, 4], + ['foo', 2, 4, 3], ]]; yield 'limit and offset' => [new TagsListFiltering(2, 1), [ - ['bar', 3, 3], - ['baz', 1, 3], + ['bar', 3, 3, 2], + ['baz', 1, 3, 2], ]]; yield 'search term' => [new TagsListFiltering(null, null, 'ba'), [ - ['bar', 3, 3], - ['baz', 1, 3], + ['bar', 3, 3, 2], + ['baz', 1, 3, 2], ]]; yield 'ASC ordering' => [ new TagsListFiltering(null, null, null, Ordering::fromTuple(['tag', 'ASC'])), $defaultList, ]; yield 'DESC ordering' => [new TagsListFiltering(null, null, null, Ordering::fromTuple(['tag', 'DESC'])), [ - ['foo', 2, 4], - ['baz', 1, 3], - ['bar', 3, 3], - ['another', 0, 0], + ['foo', 2, 4, 3], + ['baz', 1, 3, 2], + ['bar', 3, 3, 2], + ['another', 0, 0, 0], ]]; yield 'short URLs count ASC ordering' => [ new TagsListFiltering(null, null, null, Ordering::fromTuple(['shortUrlsCount', 'ASC'])), [ - ['another', 0, 0], - ['baz', 1, 3], - ['foo', 2, 4], - ['bar', 3, 3], + ['another', 0, 0, 0], + ['baz', 1, 3, 2], + ['foo', 2, 4, 3], + ['bar', 3, 3, 2], ], ]; yield 'short URLs count DESC ordering' => [ new TagsListFiltering(null, null, null, Ordering::fromTuple(['shortUrlsCount', 'DESC'])), [ - ['bar', 3, 3], - ['foo', 2, 4], - ['baz', 1, 3], - ['another', 0, 0], + ['bar', 3, 3, 2], + ['foo', 2, 4, 3], + ['baz', 1, 3, 2], + ['another', 0, 0, 0], ], ]; yield 'visits count ASC ordering' => [ new TagsListFiltering(null, null, null, Ordering::fromTuple(['visitsCount', 'ASC'])), [ - ['another', 0, 0], - ['bar', 3, 3], - ['baz', 1, 3], - ['foo', 2, 4], + ['another', 0, 0, 0], + ['bar', 3, 3, 2], + ['baz', 1, 3, 2], + ['foo', 2, 4, 3], ], ]; yield 'visits count DESC ordering' => [ new TagsListFiltering(null, null, null, Ordering::fromTuple(['visitsCount', 'DESC'])), [ - ['foo', 2, 4], - ['bar', 3, 3], - ['baz', 1, 3], - ['another', 0, 0], + ['foo', 2, 4, 3], + ['bar', 3, 3, 2], + ['baz', 1, 3, 2], + ['another', 0, 0, 0], ], ]; yield 'visits count DESC ordering and limit' => [ new TagsListFiltering(2, null, null, Ordering::fromTuple(['visitsCount', 'DESC'])), [ - ['foo', 2, 4], - ['bar', 3, 3], + ['foo', 2, 4, 3], + ['bar', 3, 3, 2], ], ]; yield 'api key' => [new TagsListFiltering(null, null, null, null, ApiKey::fromMeta( ApiKeyMeta::withRoles(RoleDefinition::forAuthoredShortUrls()), )), [ - ['bar', 2, 3], - ['baz', 1, 3], - ['foo', 1, 3], + ['bar', 2, 3, 2], + ['baz', 1, 3, 2], + ['foo', 1, 3, 2], ]]; yield 'combined' => [new TagsListFiltering(1, null, null, Ordering::fromTuple( ['shortUrls', 'DESC'], ), ApiKey::fromMeta( ApiKeyMeta::withRoles(RoleDefinition::forAuthoredShortUrls()), )), [ - ['foo', 1, 3], + ['foo', 1, 3, 2], ]]; } diff --git a/module/Rest/src/Action/Tag/TagsStatsAction.php b/module/Rest/src/Action/Tag/TagsStatsAction.php index cec8edd6..6db3c62a 100644 --- a/module/Rest/src/Action/Tag/TagsStatsAction.php +++ b/module/Rest/src/Action/Tag/TagsStatsAction.php @@ -20,7 +20,7 @@ class TagsStatsAction extends AbstractRestAction protected const ROUTE_PATH = '/tags/stats'; protected const ROUTE_ALLOWED_METHODS = [self::METHOD_GET]; - public function __construct(private TagServiceInterface $tagService) + public function __construct(private readonly TagServiceInterface $tagService) { } diff --git a/module/Rest/test-api/Action/TagsStatsTest.php b/module/Rest/test-api/Action/TagsStatsTest.php index 3b91cbf0..573f1c38 100644 --- a/module/Rest/test-api/Action/TagsStatsTest.php +++ b/module/Rest/test-api/Action/TagsStatsTest.php @@ -52,16 +52,31 @@ class TagsStatsTest extends ApiTestCase 'tag' => 'bar', 'shortUrlsCount' => 1, 'visitsCount' => 2, + 'visitsSummary' => [ + 'total' => 2, + 'nonBots' => 1, + 'bots' => 1, + ], ], [ 'tag' => 'baz', 'shortUrlsCount' => 0, 'visitsCount' => 0, + 'visitsSummary' => [ + 'total' => 0, + 'nonBots' => 0, + 'bots' => 0, + ], ], [ 'tag' => 'foo', 'shortUrlsCount' => 3, 'visitsCount' => 5, + 'visitsSummary' => [ + 'total' => 5, + 'nonBots' => 4, + 'bots' => 1, + ], ], ], [ 'currentPage' => 1, @@ -75,11 +90,21 @@ class TagsStatsTest extends ApiTestCase 'tag' => 'bar', 'shortUrlsCount' => 1, 'visitsCount' => 2, + 'visitsSummary' => [ + 'total' => 2, + 'nonBots' => 1, + 'bots' => 1, + ], ], [ 'tag' => 'baz', 'shortUrlsCount' => 0, 'visitsCount' => 0, + 'visitsSummary' => [ + 'total' => 0, + 'nonBots' => 0, + 'bots' => 0, + ], ], ], [ 'currentPage' => 1, @@ -93,11 +118,21 @@ class TagsStatsTest extends ApiTestCase 'tag' => 'bar', 'shortUrlsCount' => 1, 'visitsCount' => 2, + 'visitsSummary' => [ + 'total' => 2, + 'nonBots' => 1, + 'bots' => 1, + ], ], [ 'tag' => 'foo', 'shortUrlsCount' => 2, 'visitsCount' => 5, + 'visitsSummary' => [ + 'total' => 5, + 'nonBots' => 4, + 'bots' => 1, + ], ], ], [ 'currentPage' => 1, @@ -111,6 +146,11 @@ class TagsStatsTest extends ApiTestCase 'tag' => 'foo', 'shortUrlsCount' => 2, 'visitsCount' => 5, + 'visitsSummary' => [ + 'total' => 5, + 'nonBots' => 4, + 'bots' => 1, + ], ], ], [ 'currentPage' => 2, @@ -124,6 +164,11 @@ class TagsStatsTest extends ApiTestCase 'tag' => 'foo', 'shortUrlsCount' => 1, 'visitsCount' => 0, + 'visitsSummary' => [ + 'total' => 0, + 'nonBots' => 0, + 'bots' => 0, + ], ], ], [ 'currentPage' => 1, From ce9ec0d7386d55f48365fb000f2a2202cc0d0a98 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 2 Jan 2023 19:49:54 +0100 Subject: [PATCH 2/6] Fixed ordering in tags supporting more fields --- module/Core/src/Tag/Model/OrderableField.php | 20 ++++++----- module/Core/src/Tag/Model/TagInfo.php | 4 +-- .../Core/src/Tag/Repository/TagRepository.php | 8 ++--- .../Tag/Repository/TagRepositoryTest.php | 36 ++++++++++++++----- 4 files changed, 45 insertions(+), 23 deletions(-) diff --git a/module/Core/src/Tag/Model/OrderableField.php b/module/Core/src/Tag/Model/OrderableField.php index cb398ebb..eb802a7f 100644 --- a/module/Core/src/Tag/Model/OrderableField.php +++ b/module/Core/src/Tag/Model/OrderableField.php @@ -9,22 +9,26 @@ use function Shlinkio\Shlink\Core\camelCaseToSnakeCase; enum OrderableField: string { case TAG = 'tag'; -// case SHORT_URLS = 'shortUrls'; -// case VISITS = 'visits'; -// case NON_BOT_VISITS = 'nonBotVisits'; - + case SHORT_URLS_COUNT = 'shortUrlsCount'; + case VISITS = 'visits'; + case NON_BOT_VISITS = 'nonBotVisits'; /** @deprecated Use VISITS instead */ case VISITS_COUNT = 'visitsCount'; - /** @deprecated Use SHORT_URLS instead */ - case SHORT_URLS_COUNT = 'shortUrlsCount'; public static function isAggregateField(string $field): bool { - return $field === self::SHORT_URLS_COUNT->value || $field === self::VISITS_COUNT->value; + $parsed = self::tryFrom($field); + return $parsed !== null && $parsed !== self::TAG; } public static function toSnakeCaseValidField(?string $field): string { - return camelCaseToSnakeCase($field === self::SHORT_URLS_COUNT->value ? $field : self::VISITS_COUNT->value); + $parsed = self::tryFrom($field); + $normalized = match ($parsed) { + self::VISITS_COUNT, null => self::VISITS, + default => $parsed, + }; + + return camelCaseToSnakeCase($normalized->value); } } diff --git a/module/Core/src/Tag/Model/TagInfo.php b/module/Core/src/Tag/Model/TagInfo.php index adf5d4b1..4c0018b2 100644 --- a/module/Core/src/Tag/Model/TagInfo.php +++ b/module/Core/src/Tag/Model/TagInfo.php @@ -25,8 +25,8 @@ final class TagInfo implements JsonSerializable return new self( $data['tag'], (int) $data['shortUrlsCount'], - (int) $data['visitsCount'], - isset($data['nonBotVisitsCount']) ? (int) $data['nonBotVisitsCount'] : null, + (int) $data['visits'], + isset($data['nonBotVisits']) ? (int) $data['nonBotVisits'] : null, ); } diff --git a/module/Core/src/Tag/Repository/TagRepository.php b/module/Core/src/Tag/Repository/TagRepository.php index feaaa7d5..5dd9dcd9 100644 --- a/module/Core/src/Tag/Repository/TagRepository.php +++ b/module/Core/src/Tag/Repository/TagRepository.php @@ -72,8 +72,8 @@ class TagRepository extends EntitySpecificationRepository implements TagReposito 't.id_0 AS id', 't.name_1 AS name', 'COUNT(DISTINCT s.id) AS short_urls_count', - 'COUNT(DISTINCT v.id) AS visits_count', // Native queries require snake_case for cross-db compatibility - 'COUNT(DISTINCT v2.id) AS non_bot_visits_count', + 'COUNT(DISTINCT v.id) AS visits', // Native queries require snake_case for cross-db compatibility + 'COUNT(DISTINCT v2.id) AS non_bot_visits', ) ->from('(' . $subQb->getQuery()->getSQL() . ')', 't') // @phpstan-ignore-line ->leftJoin('t', 'short_urls_in_tags', 'st', $nativeQb->expr()->eq('t.id_0', 'st.tag_id')) @@ -108,8 +108,8 @@ class TagRepository extends EntitySpecificationRepository implements TagReposito $rsm = new ResultSetMappingBuilder($this->getEntityManager()); $rsm->addScalarResult('name', 'tag'); $rsm->addScalarResult('short_urls_count', 'shortUrlsCount'); - $rsm->addScalarResult('visits_count', 'visitsCount'); - $rsm->addScalarResult('non_bot_visits_count', 'nonBotVisitsCount'); + $rsm->addScalarResult('visits', 'visits'); + $rsm->addScalarResult('non_bot_visits', 'nonBotVisits'); return map( $this->getEntityManager()->createNativeQuery($nativeQb->getSQL(), $rsm)->getResult(), diff --git a/module/Core/test-db/Tag/Repository/TagRepositoryTest.php b/module/Core/test-db/Tag/Repository/TagRepositoryTest.php index fe030ca0..57b3a795 100644 --- a/module/Core/test-db/Tag/Repository/TagRepositoryTest.php +++ b/module/Core/test-db/Tag/Repository/TagRepositoryTest.php @@ -10,6 +10,7 @@ use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlCreation; use Shlinkio\Shlink\Core\ShortUrl\Resolver\PersistenceShortUrlRelationResolver; use Shlinkio\Shlink\Core\Tag\Entity\Tag; +use Shlinkio\Shlink\Core\Tag\Model\OrderableField; use Shlinkio\Shlink\Core\Tag\Model\TagsListFiltering; use Shlinkio\Shlink\Core\Tag\Repository\TagRepository; use Shlinkio\Shlink\Core\Visit\Entity\Visit; @@ -135,17 +136,21 @@ class TagRepositoryTest extends DatabaseTestCase ['baz', 1, 3, 2], ]]; yield 'ASC ordering' => [ - new TagsListFiltering(null, null, null, Ordering::fromTuple(['tag', 'ASC'])), + new TagsListFiltering(null, null, null, Ordering::fromTuple([OrderableField::TAG->value, 'ASC'])), $defaultList, ]; - yield 'DESC ordering' => [new TagsListFiltering(null, null, null, Ordering::fromTuple(['tag', 'DESC'])), [ + yield 'DESC ordering' => [new TagsListFiltering(null, null, null, Ordering::fromTuple( + [OrderableField::TAG->value, 'DESC'], + )), [ ['foo', 2, 4, 3], ['baz', 1, 3, 2], ['bar', 3, 3, 2], ['another', 0, 0, 0], ]]; yield 'short URLs count ASC ordering' => [ - new TagsListFiltering(null, null, null, Ordering::fromTuple(['shortUrlsCount', 'ASC'])), + new TagsListFiltering(null, null, null, Ordering::fromTuple( + [OrderableField::SHORT_URLS_COUNT->value, 'ASC'], + )), [ ['another', 0, 0, 0], ['baz', 1, 3, 2], @@ -154,7 +159,9 @@ class TagRepositoryTest extends DatabaseTestCase ], ]; yield 'short URLs count DESC ordering' => [ - new TagsListFiltering(null, null, null, Ordering::fromTuple(['shortUrlsCount', 'DESC'])), + new TagsListFiltering(null, null, null, Ordering::fromTuple( + [OrderableField::SHORT_URLS_COUNT->value, 'DESC'], + )), [ ['bar', 3, 3, 2], ['foo', 2, 4, 3], @@ -163,7 +170,18 @@ class TagRepositoryTest extends DatabaseTestCase ], ]; yield 'visits count ASC ordering' => [ - new TagsListFiltering(null, null, null, Ordering::fromTuple(['visitsCount', 'ASC'])), + new TagsListFiltering(null, null, null, Ordering::fromTuple([OrderableField::VISITS->value, 'ASC'])), + [ + ['another', 0, 0, 0], + ['bar', 3, 3, 2], + ['baz', 1, 3, 2], + ['foo', 2, 4, 3], + ], + ]; + yield 'non-bot visits count ASC ordering' => [ + new TagsListFiltering(null, null, null, Ordering::fromTuple( + [OrderableField::NON_BOT_VISITS->value, 'ASC'], + )), [ ['another', 0, 0, 0], ['bar', 3, 3, 2], @@ -172,7 +190,7 @@ class TagRepositoryTest extends DatabaseTestCase ], ]; yield 'visits count DESC ordering' => [ - new TagsListFiltering(null, null, null, Ordering::fromTuple(['visitsCount', 'DESC'])), + new TagsListFiltering(null, null, null, Ordering::fromTuple([OrderableField::VISITS->value, 'DESC'])), [ ['foo', 2, 4, 3], ['bar', 3, 3, 2], @@ -181,7 +199,7 @@ class TagRepositoryTest extends DatabaseTestCase ], ]; yield 'visits count DESC ordering and limit' => [ - new TagsListFiltering(2, null, null, Ordering::fromTuple(['visitsCount', 'DESC'])), + new TagsListFiltering(2, null, null, Ordering::fromTuple([OrderableField::VISITS_COUNT->value, 'DESC'])), [ ['foo', 2, 4, 3], ['bar', 3, 3, 2], @@ -195,11 +213,11 @@ class TagRepositoryTest extends DatabaseTestCase ['foo', 1, 3, 2], ]]; yield 'combined' => [new TagsListFiltering(1, null, null, Ordering::fromTuple( - ['shortUrls', 'DESC'], + [OrderableField::SHORT_URLS_COUNT->value, 'DESC'], ), ApiKey::fromMeta( ApiKeyMeta::withRoles(RoleDefinition::forAuthoredShortUrls()), )), [ - ['foo', 1, 3, 2], + ['bar', 2, 3, 2], ]]; } From a5929ebb297c9fbe96387fc5cb1c8c6fa81de133 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 2 Jan 2023 19:58:02 +0100 Subject: [PATCH 3/6] Added swagger docs for visits summary in tags with stats --- docs/swagger/definitions/TagInfo.json | 9 +++++++-- docs/swagger/paths/v2_tags_stats.json | 21 +++++++++++++++------ 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/docs/swagger/definitions/TagInfo.json b/docs/swagger/definitions/TagInfo.json index e881ce02..41de1068 100644 --- a/docs/swagger/definitions/TagInfo.json +++ b/docs/swagger/definitions/TagInfo.json @@ -1,5 +1,6 @@ { "type": "object", + "required": ["tag", "shortUrlsCount", "visitsSummary", "visitsCount"], "properties": { "tag": { "type": "string", @@ -9,9 +10,13 @@ "type": "number", "description": "The amount of short URLs using this tag" }, - "userAgent": { + "visitsSummary": { + "$ref": "./VisitsSummary.json" + }, + "visitsCount": { + "deprecated": true, "type": "number", - "description": "The combined amount of visits received by short URLs with this tag" + "description": "**[DEPRECATED]** Use visitsSummary.total instead" } } } diff --git a/docs/swagger/paths/v2_tags_stats.json b/docs/swagger/paths/v2_tags_stats.json index 91771335..150cf7b3 100644 --- a/docs/swagger/paths/v2_tags_stats.json +++ b/docs/swagger/paths/v2_tags_stats.json @@ -45,7 +45,7 @@ { "name": "orderBy", "in": "query", - "description": "To determine how to order the results.

**Important!** Ordering by `shortUrlsCount` or `visitsCount` has a [known performance issue](https://github.com/shlinkio/shlink/issues/1346) which makes loading a subset of the list take as much as loading the whole list.
If you plan to order by any of these fields, it's worth loading the whole list with no pagination.", + "description": "To determine how to order the results.

**Important!** Ordering by `shortUrlsCount`, `visits` or `nonBotVisits` has a [known performance issue](https://github.com/shlinkio/shlink/issues/1346) which makes loading a subset of the list take as much as loading the whole list.
If you plan to order by any of these fields, it's worth loading the whole list with no pagination.", "required": false, "schema": { "type": "string", @@ -54,8 +54,10 @@ "tag-DESC", "shortUrlsCount-ASC", "shortUrlsCount-DESC", - "visitsCount-ASC", - "visitsCount-DESC" + "visits-ASC", + "visits-DESC", + "nonBotVisits-ASC", + "nonBotVisits-DESC" ] } } @@ -73,7 +75,6 @@ "required": ["data"], "properties": { "data": { - "description": "The tag stats will be returned only if the withStats param was provided with value 'true'", "type": "array", "items": { "$ref": "../definitions/TagInfo.json" @@ -92,12 +93,20 @@ { "tag": "games", "shortUrlsCount": 10, - "visitsCount": 521 + "visitsSummary": { + "total": 521, + "nonBots": 521, + "bots": 0 + } }, { "tag": "shlink", "shortUrlsCount": 7, - "visitsCount": 1087 + "visitsSummary": { + "total": 1087, + "nonBots": 1000, + "bots": 87 + } } ], "pagination": { From 0b96a79c410cce8cf755b0d1ab3316ba961e8a85 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 2 Jan 2023 20:02:50 +0100 Subject: [PATCH 4/6] Updated async API docs --- docs/async-api/async-api.json | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/docs/async-api/async-api.json b/docs/async-api/async-api.json index 3b59e8e5..6ce83784 100644 --- a/docs/async-api/async-api.json +++ b/docs/async-api/async-api.json @@ -116,7 +116,11 @@ "format": "date-time", "description": "The date in which the short URL was created in ISO format." }, + "visitsSummary": { + "$ref": "#/components/schemas/VisitsSummary" + }, "visitsCount": { + "deprecated": true, "type": "integer", "description": "The number of visits that this short URL has received." }, @@ -149,7 +153,11 @@ "shortUrl": "https://doma.in/12C18", "longUrl": "https://store.steampowered.com", "dateCreated": "2016-08-21T20:34:16+02:00", - "visitsCount": 328, + "visitsSummary": { + "total": 328, + "nonBots": 285, + "bots": 43 + }, "tags": [ "games", "tech" @@ -189,6 +197,24 @@ } } }, + "VisitsSummary": { + "type": "object", + "required": ["total", "nonBots", "bots"], + "properties": { + "total": { + "description": "The total amount of visits", + "type": "number" + }, + "nonBots": { + "description": "The amount of visits which were not identified as bots", + "type": "number" + }, + "bots": { + "description": "The amount of visits that were identified as potential bots", + "type": "number" + } + } + }, "Visit": { "type": "object", "properties": { From fc0aba6311a01c3029e7eb013a1978d6ecf79427 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 2 Jan 2023 20:03:30 +0100 Subject: [PATCH 5/6] Updated changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4c1dc3dc..ba9b5cf3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this ## [Unreleased] ### Added * [#1632](https://github.com/shlinkio/shlink/issues/1632) Added amount of bots, non-bots and total visits to the visits summary endpoint. +* [#1633](https://github.com/shlinkio/shlink/issues/1633) Added amount of bots, non-bots and total visits to the tag stats endpoint. ### Changed * *Nothing* From 46b4a216177cc755ca98ecd542e1e13b0108f389 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 2 Jan 2023 20:17:29 +0100 Subject: [PATCH 6/6] Fixed missing null check --- module/Core/src/Tag/Model/OrderableField.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/module/Core/src/Tag/Model/OrderableField.php b/module/Core/src/Tag/Model/OrderableField.php index eb802a7f..818099de 100644 --- a/module/Core/src/Tag/Model/OrderableField.php +++ b/module/Core/src/Tag/Model/OrderableField.php @@ -23,7 +23,7 @@ enum OrderableField: string public static function toSnakeCaseValidField(?string $field): string { - $parsed = self::tryFrom($field); + $parsed = $field !== null ? self::tryFrom($field) : self::VISITS; $normalized = match ($parsed) { self::VISITS_COUNT, null => self::VISITS, default => $parsed,