diff --git a/CHANGELOG.md b/CHANGELOG.md index 6031a772..d097eced 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com), and this * Providing `excludeMaxVisitsReached=true` and/or `excludePastValidUntil=true` to the `GET /short-urls` endpoint. * Providing `--exclude-max-visits-reached` and/or `--exclude-past-valid-until` to the `short-urls:list` command. +* [#1613](https://github.com/shlinkio/shlink/issues/1613) Added amount of visits coming from bots, non-bots and total to every short URL in the short URLs list. + + Additionally, added option to order by non-bot visits, by passing `nonBotVisits-DESC` or `nonBotVisits-ASC`. + * [#1599](https://github.com/shlinkio/shlink/issues/1599) Added support for credentials on redis DSNs, either only password, or both username and password. * [#1616](https://github.com/shlinkio/shlink/issues/1616) Added support to import orphan visits when importing short URLs from another Shlink instance. * [#1519](https://github.com/shlinkio/shlink/issues/1519) Allowing to search short URLs by default domain. diff --git a/docs/swagger/definitions/ShortUrl.json b/docs/swagger/definitions/ShortUrl.json index f09e8d7b..ab66f506 100644 --- a/docs/swagger/definitions/ShortUrl.json +++ b/docs/swagger/definitions/ShortUrl.json @@ -6,6 +6,7 @@ "longUrl", "dateCreated", "visitsCount", + "visitsSummary", "tags", "meta", "domain", @@ -32,8 +33,12 @@ "description": "The date in which the short URL was created in ISO format." }, "visitsCount": { + "deprecated": true, "type": "integer", - "description": "The number of visits that this short URL has received." + "description": "**[DEPRECATED]** Use `visitsSummary.total` instead." + }, + "visitsSummary": { + "$ref": "./ShortUrlVisitsSummary.json" }, "tags": { "type": "array", diff --git a/docs/swagger/definitions/ShortUrlVisitsSummary.json b/docs/swagger/definitions/ShortUrlVisitsSummary.json new file mode 100644 index 00000000..404b7a75 --- /dev/null +++ b/docs/swagger/definitions/ShortUrlVisitsSummary.json @@ -0,0 +1,18 @@ +{ + "type": "object", + "required": ["total", "nonBots", "bots"], + "properties": { + "total": { + "description": "The total amount of visits that this short URL has received.", + "type": "integer" + }, + "nonBots": { + "description": "The amount of visits which were not identified as bots.", + "type": "integer" + }, + "bots": { + "description": "The amount of visits that were identified as potential bots.", + "type": "integer" + } + } +} diff --git a/docs/swagger/paths/v1_short-urls.json b/docs/swagger/paths/v1_short-urls.json index 05c5973a..8960234a 100644 --- a/docs/swagger/paths/v1_short-urls.json +++ b/docs/swagger/paths/v1_short-urls.json @@ -73,10 +73,12 @@ "shortCode-DESC", "dateCreated-ASC", "dateCreated-DESC", + "title-ASC", + "title-DESC", "visits-ASC", "visits-DESC", - "title-ASC", - "title-DESC" + "nonBotVisits-ASC", + "nonBotVisits-DESC" ] } }, @@ -162,7 +164,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": 328, + "bots": 0 + }, "tags": [ "games", "tech" @@ -181,7 +187,11 @@ "shortUrl": "https://doma.in/12Kb3", "longUrl": "https://shlink.io", "dateCreated": "2016-05-01T20:34:16+02:00", - "visitsCount": 1029, + "visitsSummary": { + "total": 1029, + "nonBots": 900, + "bots": 129 + }, "tags": [ "shlink" ], @@ -199,7 +209,11 @@ "shortUrl": "https://example.com/123bA", "longUrl": "https://www.google.com", "dateCreated": "2015-10-01T20:34:16+02:00", - "visitsCount": 25, + "visitsSummary": { + "total": 25, + "nonBots": 0, + "bots": 25 + }, "tags": [], "meta": { "validSince": "2017-01-21T00:00:00+02:00", @@ -307,7 +321,11 @@ "shortUrl": "https://doma.in/12C18", "longUrl": "https://store.steampowered.com", "dateCreated": "2016-08-21T20:34:16+02:00", - "visitsCount": 0, + "visitsSummary": { + "total": 0, + "nonBots": 0, + "bots": 0 + }, "tags": [ "games", "tech" diff --git a/docs/swagger/paths/v1_short-urls_shorten.json b/docs/swagger/paths/v1_short-urls_shorten.json index aa26fa1b..254a88f2 100644 --- a/docs/swagger/paths/v1_short-urls_shorten.json +++ b/docs/swagger/paths/v1_short-urls_shorten.json @@ -55,7 +55,11 @@ "shortUrl": "https://doma.in/abc123", "shortCode": "abc123", "dateCreated": "2016-08-21T20:34:16+02:00", - "visitsCount": 0, + "visitsSummary": { + "total": 0, + "nonBots": 0, + "bots": 0 + }, "tags": [ "games", "tech" diff --git a/docs/swagger/paths/v1_short-urls_{shortCode}.json b/docs/swagger/paths/v1_short-urls_{shortCode}.json index 1b001cc9..00577f4f 100644 --- a/docs/swagger/paths/v1_short-urls_{shortCode}.json +++ b/docs/swagger/paths/v1_short-urls_{shortCode}.json @@ -41,7 +41,11 @@ "shortUrl": "https://doma.in/12Kb3", "longUrl": "https://shlink.io", "dateCreated": "2016-05-01T20:34:16+02:00", - "visitsCount": 1029, + "visitsSummary": { + "total": 1029, + "nonBots": 820, + "bots": 209 + }, "tags": [ "shlink" ], @@ -159,7 +163,11 @@ "shortUrl": "https://doma.in/12Kb3", "longUrl": "https://shlink.io", "dateCreated": "2016-05-01T20:34:16+02:00", - "visitsCount": 1029, + "visitsSummary": { + "total": 1029, + "nonBots": 900, + "bots": 129 + }, "tags": [ "shlink" ], diff --git a/module/Core/src/ShortUrl/Entity/ShortUrl.php b/module/Core/src/ShortUrl/Entity/ShortUrl.php index c7f10b75..0ebdeb24 100644 --- a/module/Core/src/ShortUrl/Entity/ShortUrl.php +++ b/module/Core/src/ShortUrl/Entity/ShortUrl.php @@ -33,9 +33,9 @@ class ShortUrl extends AbstractEntity private string $longUrl; private string $shortCode; private Chronos $dateCreated; - /** @var Collection|Visit[] */ + /** @var Collection */ private Collection $visits; - /** @var Collection|Tag[] */ + /** @var Collection */ private Collection $tags; private ?Chronos $validSince = null; private ?Chronos $validUntil = null; @@ -141,7 +141,7 @@ class ShortUrl extends AbstractEntity } /** - * @return Collection|Tag[] + * @return Collection */ public function getTags(): Collection { @@ -168,6 +168,12 @@ class ShortUrl extends AbstractEntity return count($this->visits); } + public function nonBotVisitsCount(): int + { + $criteria = Criteria::create()->where(Criteria::expr()->eq('potentialBot', false)); + return count($this->visits->matching($criteria)); + } + public function mostRecentImportedVisitDate(): ?Chronos { /** @var Selectable $visits */ @@ -183,7 +189,7 @@ class ShortUrl extends AbstractEntity } /** - * @param Collection|Visit[] $visits + * @param Collection $visits * @internal */ public function setVisits(Collection $visits): self diff --git a/module/Core/src/ShortUrl/Model/OrderableField.php b/module/Core/src/ShortUrl/Model/OrderableField.php new file mode 100644 index 00000000..1c1c6338 --- /dev/null +++ b/module/Core/src/ShortUrl/Model/OrderableField.php @@ -0,0 +1,37 @@ + $field->value); + } + + public static function isBasicField(string $value): bool + { + return contains( + [self::LONG_URL->value, self::SHORT_CODE->value, self::DATE_CREATED->value, self::TITLE->value], + $value, + ); + } + + public static function isVisitsField(string $value): bool + { + return $value === self::VISITS->value || $value === self::NON_BOT_VISITS->value; + } +} diff --git a/module/Core/src/ShortUrl/Model/ShortUrlsParams.php b/module/Core/src/ShortUrl/Model/ShortUrlsParams.php index e053a283..88e20aa7 100644 --- a/module/Core/src/ShortUrl/Model/ShortUrlsParams.php +++ b/module/Core/src/ShortUrl/Model/ShortUrlsParams.php @@ -14,7 +14,6 @@ use function Shlinkio\Shlink\Core\normalizeOptionalDate; final class ShortUrlsParams { - public const ORDERABLE_FIELDS = ['longUrl', 'shortCode', 'dateCreated', 'title', 'visits']; public const DEFAULT_ITEMS_PER_PAGE = 10; private function __construct( diff --git a/module/Core/src/ShortUrl/Model/Validation/ShortUrlsParamsInputFilter.php b/module/Core/src/ShortUrl/Model/Validation/ShortUrlsParamsInputFilter.php index 3bdea8e2..cb120e8e 100644 --- a/module/Core/src/ShortUrl/Model/Validation/ShortUrlsParamsInputFilter.php +++ b/module/Core/src/ShortUrl/Model/Validation/ShortUrlsParamsInputFilter.php @@ -8,7 +8,7 @@ use Laminas\InputFilter\InputFilter; use Laminas\Validator\InArray; use Shlinkio\Shlink\Common\Paginator\Paginator; use Shlinkio\Shlink\Common\Validation; -use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlsParams; +use Shlinkio\Shlink\Core\ShortUrl\Model\OrderableField; use Shlinkio\Shlink\Core\ShortUrl\Model\TagsMode; class ShortUrlsParamsInputFilter extends InputFilter @@ -51,7 +51,7 @@ class ShortUrlsParamsInputFilter extends InputFilter ])); $this->add($tagsMode); - $this->add($this->createOrderByInput(self::ORDER_BY, ShortUrlsParams::ORDERABLE_FIELDS)); + $this->add($this->createOrderByInput(self::ORDER_BY, OrderableField::values())); $this->add($this->createBooleanInput(self::EXCLUDE_MAX_VISITS_REACHED, false)); $this->add($this->createBooleanInput(self::EXCLUDE_PAST_VALID_UNTIL, false)); diff --git a/module/Core/src/ShortUrl/Repository/ShortUrlListRepository.php b/module/Core/src/ShortUrl/Repository/ShortUrlListRepository.php index 35d7996f..e014ac64 100644 --- a/module/Core/src/ShortUrl/Repository/ShortUrlListRepository.php +++ b/module/Core/src/ShortUrl/Repository/ShortUrlListRepository.php @@ -10,13 +10,13 @@ use Doctrine\ORM\QueryBuilder; use Happyr\DoctrineSpecification\Repository\EntitySpecificationRepository; use Shlinkio\Shlink\Common\Doctrine\Type\ChronosDateTimeType; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; +use Shlinkio\Shlink\Core\ShortUrl\Model\OrderableField; use Shlinkio\Shlink\Core\ShortUrl\Model\TagsMode; use Shlinkio\Shlink\Core\ShortUrl\Persistence\ShortUrlsCountFiltering; use Shlinkio\Shlink\Core\ShortUrl\Persistence\ShortUrlsListFiltering; use Shlinkio\Shlink\Core\Visit\Entity\Visit; use function array_column; -use function Functional\contains; use function sprintf; class ShortUrlListRepository extends EntitySpecificationRepository implements ShortUrlListRepositoryInterface @@ -31,11 +31,10 @@ class ShortUrlListRepository extends EntitySpecificationRepository implements Sh ->setMaxResults($filtering->limit) ->setFirstResult($filtering->offset); - // In case the ordering has been specified, the query could be more complex. Process it $this->processOrderByForList($qb, $filtering); $result = $qb->getQuery()->getResult(); - if ($filtering->orderBy->field === 'visits') { + if (OrderableField::isVisitsField($filtering->orderBy->field ?? '')) { return array_column($result, 0); } @@ -45,23 +44,28 @@ class ShortUrlListRepository extends EntitySpecificationRepository implements Sh private function processOrderByForList(QueryBuilder $qb, ShortUrlsListFiltering $filtering): void { // With no explicit order by, fallback to dateCreated-DESC - if (! $filtering->orderBy->hasOrderField()) { + $fieldName = $filtering->orderBy->field; + if ($fieldName === null) { $qb->orderBy('s.dateCreated', 'DESC'); return; } - $fieldName = $filtering->orderBy->field; $order = $filtering->orderBy->direction; - if ($fieldName === 'visits') { + if (OrderableField::isBasicField($fieldName)) { + $qb->orderBy('s.' . $fieldName, $order); + } elseif (OrderableField::isVisitsField($fieldName)) { // FIXME This query is inefficient. // Diagnostic: It might need to use a sub-query, as done with the tags list query. $qb->addSelect('COUNT(DISTINCT v)') - ->leftJoin('s.visits', 'v') - ->groupBy('s') - ->orderBy('COUNT(DISTINCT v)', $order); - } elseif (contains(['longUrl', 'shortCode', 'dateCreated', 'title'], $fieldName)) { - $qb->orderBy('s.' . $fieldName, $order); + ->leftJoin('s.visits', 'v', Join::WITH, $qb->expr()->andX( + $qb->expr()->eq('v.shortUrl', 's'), + $fieldName === OrderableField::NON_BOT_VISITS->value + ? $qb->expr()->eq('v.potentialBot', 'false') + : null, + )) + ->groupBy('s') + ->orderBy('COUNT(DISTINCT v)', $order); } } diff --git a/module/Core/src/ShortUrl/ShortUrlListService.php b/module/Core/src/ShortUrl/ShortUrlListService.php index 5287f14d..d83647f0 100644 --- a/module/Core/src/ShortUrl/ShortUrlListService.php +++ b/module/Core/src/ShortUrl/ShortUrlListService.php @@ -6,6 +6,7 @@ namespace Shlinkio\Shlink\Core\ShortUrl; use Shlinkio\Shlink\Common\Paginator\Paginator; use Shlinkio\Shlink\Core\Options\UrlShortenerOptions; +use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlsParams; use Shlinkio\Shlink\Core\ShortUrl\Paginator\Adapter\ShortUrlRepositoryAdapter; use Shlinkio\Shlink\Core\ShortUrl\Repository\ShortUrlListRepositoryInterface; @@ -19,6 +20,9 @@ class ShortUrlListService implements ShortUrlListServiceInterface ) { } + /** + * @return ShortUrl[]|Paginator + */ public function listShortUrls(ShortUrlsParams $params, ?ApiKey $apiKey = null): Paginator { $defaultDomain = $this->urlShortenerOptions->domain['hostname'] ?? ''; diff --git a/module/Core/src/ShortUrl/Transformer/ShortUrlDataTransformer.php b/module/Core/src/ShortUrl/Transformer/ShortUrlDataTransformer.php index 262989ce..bd82cd9d 100644 --- a/module/Core/src/ShortUrl/Transformer/ShortUrlDataTransformer.php +++ b/module/Core/src/ShortUrl/Transformer/ShortUrlDataTransformer.php @@ -13,7 +13,7 @@ use function Functional\invoke_if; class ShortUrlDataTransformer implements DataTransformerInterface { - public function __construct(private ShortUrlStringifierInterface $stringifier) + public function __construct(private readonly ShortUrlStringifierInterface $stringifier) { } @@ -27,13 +27,16 @@ class ShortUrlDataTransformer implements DataTransformerInterface 'shortUrl' => $this->stringifier->stringify($shortUrl), 'longUrl' => $shortUrl->getLongUrl(), 'dateCreated' => $shortUrl->getDateCreated()->toAtomString(), - 'visitsCount' => $shortUrl->getVisitsCount(), 'tags' => invoke($shortUrl->getTags(), '__toString'), 'meta' => $this->buildMeta($shortUrl), 'domain' => $shortUrl->getDomain(), 'title' => $shortUrl->title(), 'crawlable' => $shortUrl->crawlable(), 'forwardQuery' => $shortUrl->forwardQuery(), + 'visitsSummary' => $this->buildVisitsSummary($shortUrl), + + // Deprecated + 'visitsCount' => $shortUrl->getVisitsCount(), ]; } @@ -49,4 +52,16 @@ class ShortUrlDataTransformer implements DataTransformerInterface 'maxVisits' => $maxVisits, ]; } + + private function buildVisitsSummary(ShortUrl $shortUrl): array + { + $totalVisits = $shortUrl->getVisitsCount(); + $nonBotVisits = $shortUrl->nonBotVisitsCount(); + + return [ + 'total' => $totalVisits, + 'nonBots' => $nonBotVisits, + 'bots' => $totalVisits - $nonBotVisits, + ]; + } } diff --git a/module/Core/test-db/ShortUrl/Repository/ShortUrlListRepositoryTest.php b/module/Core/test-db/ShortUrl/Repository/ShortUrlListRepositoryTest.php index 76c87bd0..d55c301c 100644 --- a/module/Core/test-db/ShortUrl/Repository/ShortUrlListRepositoryTest.php +++ b/module/Core/test-db/ShortUrl/Repository/ShortUrlListRepositoryTest.php @@ -10,6 +10,7 @@ use ReflectionObject; use Shlinkio\Shlink\Common\Util\DateRange; use Shlinkio\Shlink\Core\Model\Ordering; use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl; +use Shlinkio\Shlink\Core\ShortUrl\Model\OrderableField; use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlCreation; use Shlinkio\Shlink\Core\ShortUrl\Model\TagsMode; use Shlinkio\Shlink\Core\ShortUrl\Persistence\ShortUrlsCountFiltering; @@ -21,6 +22,8 @@ use Shlinkio\Shlink\Core\Visit\Model\Visitor; use Shlinkio\Shlink\TestUtils\DbTest\DatabaseTestCase; use function count; +use function Functional\map; +use function range; class ShortUrlListRepositoryTest extends DatabaseTestCase { @@ -56,12 +59,23 @@ class ShortUrlListRepositoryTest extends DatabaseTestCase $this->getEntityManager()->persist($foo); $bar = ShortUrl::withLongUrl('bar'); - $visit = Visit::forValidShortUrl($bar, Visitor::emptyInstance()); - $this->getEntityManager()->persist($visit); - $bar->setVisits(new ArrayCollection([$visit])); + $visits = map(range(0, 5), function () use ($bar) { + $visit = Visit::forValidShortUrl($bar, Visitor::botInstance()); + $this->getEntityManager()->persist($visit); + + return $visit; + }); + $bar->setVisits(new ArrayCollection($visits)); $this->getEntityManager()->persist($bar); $foo2 = ShortUrl::withLongUrl('foo_2'); + $visits2 = map(range(0, 3), function () use ($foo2) { + $visit = Visit::forValidShortUrl($foo2, Visitor::emptyInstance()); + $this->getEntityManager()->persist($visit); + + return $visit; + }); + $foo2->setVisits(new ArrayCollection($visits2)); $ref = new ReflectionObject($foo2); $dateProp = $ref->getProperty('dateCreated'); $dateProp->setAccessible(true); @@ -95,11 +109,19 @@ class ShortUrlListRepositoryTest extends DatabaseTestCase self::assertCount(1, $this->repo->findList(new ShortUrlsListFiltering(2, 2, Ordering::emptyInstance()))); $result = $this->repo->findList( - new ShortUrlsListFiltering(null, null, Ordering::fromTuple(['visits', 'DESC'])), + new ShortUrlsListFiltering(null, null, Ordering::fromTuple([OrderableField::VISITS->value, 'DESC'])), ); self::assertCount(3, $result); self::assertSame($bar, $result[0]); + $result = $this->repo->findList( + new ShortUrlsListFiltering(null, null, Ordering::fromTuple( + [OrderableField::NON_BOT_VISITS->value, 'DESC'], + )), + ); + self::assertCount(3, $result); + self::assertSame($foo2, $result[0]); + $result = $this->repo->findList( new ShortUrlsListFiltering(null, null, Ordering::emptyInstance(), null, [], null, DateRange::until( Chronos::now()->subDays(2), diff --git a/module/Core/test/EventDispatcher/PublishingUpdatesGeneratorTest.php b/module/Core/test/EventDispatcher/PublishingUpdatesGeneratorTest.php index 3ac690d0..c7a4ecd0 100644 --- a/module/Core/test/EventDispatcher/PublishingUpdatesGeneratorTest.php +++ b/module/Core/test/EventDispatcher/PublishingUpdatesGeneratorTest.php @@ -63,6 +63,11 @@ class PublishingUpdatesGeneratorTest extends TestCase 'title' => $title, 'crawlable' => false, 'forwardQuery' => true, + 'visitsSummary' => [ + 'total' => 0, + 'nonBots' => 0, + 'bots' => 0, + ], ], 'visit' => [ 'referer' => '', @@ -139,6 +144,11 @@ class PublishingUpdatesGeneratorTest extends TestCase 'title' => $shortUrl->title(), 'crawlable' => false, 'forwardQuery' => true, + 'visitsSummary' => [ + 'total' => 0, + 'nonBots' => 0, + 'bots' => 0, + ], ]], $update->payload); } } diff --git a/module/Rest/test-api/Action/ListShortUrlsTest.php b/module/Rest/test-api/Action/ListShortUrlsTest.php index 63664a6d..d3a515c1 100644 --- a/module/Rest/test-api/Action/ListShortUrlsTest.php +++ b/module/Rest/test-api/Action/ListShortUrlsTest.php @@ -18,6 +18,11 @@ class ListShortUrlsTest extends ApiTestCase 'longUrl' => 'https://shlink.io', 'dateCreated' => '2018-05-01T00:00:00+00:00', 'visitsCount' => 3, + 'visitsSummary' => [ + 'total' => 3, + 'nonBots' => 3, + 'bots' => 0, + ], 'tags' => ['foo'], 'meta' => [ 'validSince' => null, @@ -35,6 +40,11 @@ class ListShortUrlsTest extends ApiTestCase 'longUrl' => 'https://shlink.io/documentation/', 'dateCreated' => '2018-05-01T00:00:00+00:00', 'visitsCount' => 2, + 'visitsSummary' => [ + 'total' => 2, + 'nonBots' => 2, + 'bots' => 0, + ], 'tags' => [], 'meta' => [ 'validSince' => null, @@ -52,6 +62,11 @@ class ListShortUrlsTest extends ApiTestCase 'longUrl' => 'https://google.com', 'dateCreated' => '2018-10-20T00:00:00+00:00', 'visitsCount' => 0, + 'visitsSummary' => [ + 'total' => 0, + 'nonBots' => 0, + 'bots' => 0, + ], 'tags' => [], 'meta' => [ 'validSince' => null, @@ -71,6 +86,11 @@ class ListShortUrlsTest extends ApiTestCase . '/acmailer-7-0-the-most-important-release-in-a-long-time/', 'dateCreated' => '2019-01-01T00:00:10+00:00', 'visitsCount' => 2, + 'visitsSummary' => [ + 'total' => 2, + 'nonBots' => 1, + 'bots' => 1, + ], 'tags' => ['bar', 'foo'], 'meta' => [ 'validSince' => '2020-05-01T00:00:00+00:00', @@ -88,6 +108,11 @@ class ListShortUrlsTest extends ApiTestCase 'longUrl' => 'https://shlink.io', 'dateCreated' => '2019-01-01T00:00:20+00:00', 'visitsCount' => 0, + 'visitsSummary' => [ + 'total' => 0, + 'nonBots' => 0, + 'bots' => 0, + ], 'tags' => [], 'meta' => [ 'validSince' => null, @@ -107,6 +132,11 @@ class ListShortUrlsTest extends ApiTestCase . '/considerations-to-properly-use-open-source-software-projects/', 'dateCreated' => '2019-01-01T00:00:30+00:00', 'visitsCount' => 0, + 'visitsSummary' => [ + 'total' => 0, + 'nonBots' => 0, + 'bots' => 0, + ], 'tags' => ['foo'], 'meta' => [ 'validSince' => null,