From 961178fd82d3c91a06f91bafd8bf998b5e2fd0fd Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Mon, 2 Jan 2023 19:28:32 +0100 Subject: [PATCH] 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,