Merge pull request #1652 from acelaya-forks/feature/extended-tags-stats

Feature/extended tags stats
This commit is contained in:
Alejandro Celaya 2023-01-02 20:25:50 +01:00 committed by GitHub
commit e3397a7c90
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
13 changed files with 240 additions and 73 deletions

View file

@ -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*

View file

@ -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": {

View file

@ -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"
}
}
}

View file

@ -45,7 +45,7 @@
{
"name": "orderBy",
"in": "query",
"description": "To determine how to order the results.<br /><br />**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.<br />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.<br /><br />**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.<br />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": {

View file

@ -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],
);
}
}

View file

@ -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);

View file

@ -0,0 +1,34 @@
<?php
declare(strict_types=1);
namespace Shlinkio\Shlink\Core\Tag\Model;
use function Shlinkio\Shlink\Core\camelCaseToSnakeCase;
enum OrderableField: string
{
case TAG = 'tag';
case SHORT_URLS_COUNT = 'shortUrlsCount';
case VISITS = 'visits';
case NON_BOT_VISITS = 'nonBotVisits';
/** @deprecated Use VISITS instead */
case VISITS_COUNT = 'visitsCount';
public static function isAggregateField(string $field): bool
{
$parsed = self::tryFrom($field);
return $parsed !== null && $parsed !== self::TAG;
}
public static function toSnakeCaseValidField(?string $field): string
{
$parsed = $field !== null ? self::tryFrom($field) : self::VISITS;
$normalized = match ($parsed) {
self::VISITS_COUNT, null => self::VISITS,
default => $parsed,
};
return camelCaseToSnakeCase($normalized->value);
}
}

View file

@ -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['visits'],
isset($data['nonBotVisits']) ? (int) $data['nonBotVisits'] : 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,
];
}
}

View file

@ -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', // 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'))
->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);
}
@ -106,7 +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('visits', 'visits');
$rsm->addScalarResult('non_bot_visits', 'nonBotVisits');
return map(
$this->getEntityManager()->createNativeQuery($nativeQb->getSQL(), $rsm)->getResult(),

View file

@ -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)
{
}

View file

@ -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;
@ -73,7 +74,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 +82,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 +101,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 +112,112 @@ 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'])),
new TagsListFiltering(null, null, null, Ordering::fromTuple([OrderableField::TAG->value, '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],
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],
['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'])),
new TagsListFiltering(null, null, null, Ordering::fromTuple(
[OrderableField::SHORT_URLS_COUNT->value, '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'])),
new TagsListFiltering(null, null, null, Ordering::fromTuple([OrderableField::VISITS->value, '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 '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],
['baz', 1, 3, 2],
['foo', 2, 4, 3],
],
];
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],
['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'])),
new TagsListFiltering(2, null, null, Ordering::fromTuple([OrderableField::VISITS_COUNT->value, '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'],
[OrderableField::SHORT_URLS_COUNT->value, 'DESC'],
), ApiKey::fromMeta(
ApiKeyMeta::withRoles(RoleDefinition::forAuthoredShortUrls()),
)), [
['foo', 1, 3],
['bar', 2, 3, 2],
]];
}

View file

@ -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)
{
}

View file

@ -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,