From c7a2f499e001f2685493c2257aab3b66b98ba963 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Fri, 16 Dec 2022 19:42:29 +0100 Subject: [PATCH] Added support to order short URLs list by amount of non-bot visits --- docs/swagger/paths/v1_short-urls.json | 6 ++- .../src/ShortUrl/Model/OrderableField.php | 37 +++++++++++++++++++ .../src/ShortUrl/Model/ShortUrlsParams.php | 1 - .../Validation/ShortUrlsParamsInputFilter.php | 4 +- .../Repository/ShortUrlListRepository.php | 26 +++++++------ .../Repository/ShortUrlListRepositoryTest.php | 30 +++++++++++++-- 6 files changed, 84 insertions(+), 20 deletions(-) create mode 100644 module/Core/src/ShortUrl/Model/OrderableField.php diff --git a/docs/swagger/paths/v1_short-urls.json b/docs/swagger/paths/v1_short-urls.json index 4773bddf..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" ] } }, 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/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),