OilfieldToolsNet/shlink
4
0
Fork 0
mirror of https://github.com/shlinkio/shlink.git synced 2025-01-26 16:26:39 -06:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #1723 from acelaya-forks/feature/tags-list-performance-join-tags

Browse Source 
Feature/tags list performance join tags
This commit is contained in:
Alejandro Celaya 2023-03-16 21:57:43 +01:00 committed by GitHub
commit 11f94b8306
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 107 additions and 126 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

28
data/migrations/Version20230303164233.php Normal file
View File

@ -0,0 +1,28 @@
<?php
declare(strict_types=1);
namespace ShlinkMigrations;
use Doctrine\DBAL\Platforms\MySQLPlatform;
use Doctrine\DBAL\Schema\Schema;
use Doctrine\Migrations\AbstractMigration;
final class Version20230303164233 extends AbstractMigration
{
private const INDEX_NAME = 'visits_potential_bot_IDX';
public function up(Schema $schema): void
{
$visits = $schema->getTable('visits');
$this->skipIf($visits->hasIndex(self::INDEX_NAME));
$visits->dropIndex('IDX_visits_potential_bot'); // Old index
$visits->addIndex(['potential_bot'], self::INDEX_NAME);
}
public function isTransactional(): bool
{
return ! ($this->connection->getDatabasePlatform() instanceof MySQLPlatform);
}
}

3
module/CLI/src/Command/Api/GenerateKeyCommand.php
View File

@ -9,6 +9,7 @@ use Shlinkio\Shlink\CLI\ApiKey\RoleResolverInterface;
use Shlinkio\Shlink\CLI\Util\ExitCodes;
use Shlinkio\Shlink\CLI\Util\ShlinkTable;
use Shlinkio\Shlink\Rest\ApiKey\Role;
use Shlinkio\Shlink\Rest\Entity\ApiKey;
use Shlinkio\Shlink\Rest\Service\ApiKeyServiceInterface;
use Symfony\Component\Console\Command\Command;
use Symfony\Component\Console\Input\InputInterface;
@ -99,7 +100,7 @@ class GenerateKeyCommand extends Command
$io = new SymfonyStyle($input, $output);
$io->success(sprintf('Generated API key: "%s"', $apiKey->toString()));
if (! $apiKey->isAdmin()) {
if (! ApiKey::isAdmin($apiKey)) {
ShlinkTable::default($io)->render(
['Role name', 'Role metadata'],
$apiKey->mapRoles(fn (Role $role, array $meta) => [$role->value, arrayToString($meta, 0)]),

2
module/CLI/src/Command/Api/ListKeysCommand.php
View File

@ -59,7 +59,7 @@ class ListKeysCommand extends Command
$rowData[] = sprintf($messagePattern, $this->getEnabledSymbol($apiKey));
}
$rowData[] = $expiration?->toAtomString() ?? '-';
$rowData[] = $apiKey->isAdmin() ? 'Admin' : implode("\n", $apiKey->mapRoles(
$rowData[] = ApiKey::isAdmin($apiKey) ? 'Admin' : implode("\n", $apiKey->mapRoles(
fn (Role $role, array $meta) =>
empty($meta)
? $role->toFriendlyName()

16
module/Core/src/Tag/Model/OrderableField.php
View File

@ -4,8 +4,6 @@ declare(strict_types=1);
namespace Shlinkio\Shlink\Core\Tag\Model;
use function Shlinkio\Shlink\Core\camelCaseToSnakeCase;
enum OrderableField: string
{
case TAG = 'tag';
@ -15,20 +13,12 @@ enum OrderableField: string
/** @deprecated Use VISITS instead */
case VISITS_COUNT = 'visitsCount';
public static function isAggregateField(string $field): bool
public static function toSnakeCaseValidField(?string $field): self
{
$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) {
$parsed = $field !== null ? self::tryFrom($field) : self::TAG;
return match ($parsed) {
self::VISITS_COUNT, null => self::VISITS,
default => $parsed,
};
return camelCaseToSnakeCase($normalized->value);
}
}

141
module/Core/src/Tag/Repository/TagRepository.php
View File

@ -15,10 +15,11 @@ use Shlinkio\Shlink\Core\Tag\Model\TagsListFiltering;
use Shlinkio\Shlink\Core\Tag\Spec\CountTagsWithName;
use Shlinkio\Shlink\Rest\ApiKey\Role;
use Shlinkio\Shlink\Rest\ApiKey\Spec\WithApiKeySpecsEnsuringJoin;
use Shlinkio\Shlink\Rest\ApiKey\Spec\WithInlinedApiKeySpecsEnsuringJoin;
use Shlinkio\Shlink\Rest\Entity\ApiKey;
use function Functional\each;
use function Functional\map;
use function Shlinkio\Shlink\Core\camelCaseToSnakeCase;
use const PHP_INT_MAX;
@ -42,106 +43,90 @@ class TagRepository extends EntitySpecificationRepository implements TagReposito
*/
public function findTagsWithInfo(?TagsListFiltering $filtering = null): array
{
$orderField = $filtering?->orderBy?->field;
$orderDir = $filtering?->orderBy?->direction;
$orderMainQuery = $orderField !== null && OrderableField::isAggregateField($orderField);
$subQb = $this->createQueryBuilder('t');
$subQb->select('t.id', 't.name');
if (! $orderMainQuery) {
$subQb->orderBy('t.name', $orderDir ?? 'ASC')
->setMaxResults($filtering?->limit ?? PHP_INT_MAX)
->setFirstResult($filtering?->offset ?? 0);
// TODO Check if applying limit/offset ot visits sub-queries is needed with large amounts of tags
}
$conn = $this->getEntityManager()->getConnection();
$buildVisitsSubQuery = static function (bool $excludeBots, string $aggregateAlias) use ($conn) {
$visitsSubQuery = $conn->createQueryBuilder();
$commonJoinCondition = $visitsSubQuery->expr()->eq('v.short_url_id', 's.id');
$visitsJoin = ! $excludeBots
? $commonJoinCondition
: $visitsSubQuery->expr()->and(
$commonJoinCondition,
$visitsSubQuery->expr()->eq('v.potential_bot', $conn->quote('0')),
);
return $visitsSubQuery
->select('st.tag_id AS tag_id', 'COUNT(DISTINCT v.id) AS ' . $aggregateAlias)
->from('visits', 'v')
->join('v', 'short_urls', 's', $visitsJoin) // @phpstan-ignore-line
->join('s', 'short_urls_in_tags', 'st', $visitsSubQuery->expr()->eq('st.short_url_id', 's.id'))
->groupBy('st.tag_id');
};
$allVisitsSubQuery = $buildVisitsSubQuery(false, 'visits');
$nonBotVisitsSubQuery = $buildVisitsSubQuery(true, 'non_bot_visits');
$searchTerm = $filtering?->searchTerm;
if ($searchTerm !== null) {
$subQb->andWhere($subQb->expr()->like('t.name', $conn->quote('%' . $searchTerm . '%')));
// TODO Check if applying this to all sub-queries makes it faster or slower
}
$orderField = OrderableField::toSnakeCaseValidField($filtering?->orderBy?->field);
$orderDir = $filtering?->orderBy?->direction ?? 'ASC';
$apiKey = $filtering?->apiKey;
$applyApiKeyToNativeQuery = static fn (?ApiKey $apiKey, NativeQueryBuilder $nativeQueryBuilder) =>
$conn = $this->getEntityManager()->getConnection();
$applyApiKeyToNativeQb = static fn (NativeQueryBuilder $qb) =>
$apiKey?->mapRoles(static fn (Role $role, array $meta) => match ($role) {
Role::DOMAIN_SPECIFIC => $nativeQueryBuilder->andWhere(
$nativeQueryBuilder->expr()->eq('s.domain_id', $conn->quote(Role::domainIdFromMeta($meta))),
Role::DOMAIN_SPECIFIC => $qb->andWhere(
$qb->expr()->eq('s.domain_id', $conn->quote(Role::domainIdFromMeta($meta))),
),
Role::AUTHORED_SHORT_URLS => $nativeQueryBuilder->andWhere(
$nativeQueryBuilder->expr()->eq('s.author_api_key_id', $conn->quote($apiKey->getId())),
Role::AUTHORED_SHORT_URLS => $qb->andWhere(
$qb->expr()->eq('s.author_api_key_id', $conn->quote($apiKey->getId())),
),
});
// For admins and when no API key is present, we'll return tags which are not linked to any short URL
$joiningMethod = ApiKey::isAdmin($apiKey) ? 'leftJoin' : 'join';
$tagsSubQb = $conn->createQueryBuilder();
$tagsSubQb
->select('t.id AS tag_id', 't.name AS tag', 'COUNT(DISTINCT s.id) AS short_urls_count')
->from('tags', 't')
->groupBy('t.id', 't.name')
->{$joiningMethod}('t', 'short_urls_in_tags', 'st', $tagsSubQb->expr()->eq('st.tag_id', 't.id'))
->{$joiningMethod}('st', 'short_urls', 's', $tagsSubQb->expr()->eq('st.short_url_id', 's.id'));
$searchTerm = $filtering?->searchTerm;
if ($searchTerm !== null) {
$tagsSubQb->andWhere($tagsSubQb->expr()->like('t.name', $conn->quote('%' . $searchTerm . '%')));
}
$buildVisitsSubQb = static function (bool $excludeBots, string $aggregateAlias) use ($conn) {
$visitsSubQb = $conn->createQueryBuilder();
$commonJoinCondition = $visitsSubQb->expr()->eq('v.short_url_id', 's.id');
$visitsJoin = ! $excludeBots
? $commonJoinCondition
: $visitsSubQb->expr()->and(
$commonJoinCondition,
$visitsSubQb->expr()->eq('v.potential_bot', $conn->quote('0')),
);
return $visitsSubQb
->select('st.tag_id AS tag_id', 'COUNT(DISTINCT v.id) AS ' . $aggregateAlias)
->from('visits', 'v')
->join('v', 'short_urls', 's', $visitsJoin) // @phpstan-ignore-line
->join('s', 'short_urls_in_tags', 'st', $visitsSubQb->expr()->eq('st.short_url_id', 's.id'))
->groupBy('st.tag_id');
};
$allVisitsSubQb = $buildVisitsSubQb(false, 'visits');
$nonBotVisitsSubQb = $buildVisitsSubQb(true, 'non_bot_visits');
// Apply API key specification to all sub-queries
$this->applySpecification($subQb, new WithInlinedApiKeySpecsEnsuringJoin($apiKey), 't');
$applyApiKeyToNativeQuery($apiKey, $allVisitsSubQuery);
$applyApiKeyToNativeQuery($apiKey, $nonBotVisitsSubQuery);
each([$tagsSubQb, $allVisitsSubQb, $nonBotVisitsSubQb], $applyApiKeyToNativeQb);
// A native query builder needs to be used here, because DQL and ORM query builders do not support
// sub-queries at "from" and "join" level.
// If no sub-query is used, the whole list is loaded even with pagination, making it very inefficient.
$nativeQb = $conn->createQueryBuilder();
$nativeQb
$mainQb = $conn->createQueryBuilder();
$mainQb
->select(
't.id_0 AS id',
't.name_1 AS name',
't.tag AS tag',
'COALESCE(v.visits, 0) AS visits', // COALESCE required for postgres to properly order
'COALESCE(v2.non_bot_visits, 0) AS non_bot_visits', // COALESCE required for postgres to properly order
'COUNT(DISTINCT s.id) AS short_urls_count',
'COALESCE(b.non_bot_visits, 0) AS non_bot_visits',
'COALESCE(t.short_urls_count, 0) AS short_urls_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('t', '(' . $allVisitsSubQuery->getSQL() . ')', 'v', $nativeQb->expr()->eq('t.id_0', 'v.tag_id'))
->leftJoin('t', '(' . $nonBotVisitsSubQuery->getSQL() . ')', 'v2', $nativeQb->expr()->eq(
't.id_0',
'v2.tag_id',
))
->groupBy('t.id_0', 't.name_1', 'v.visits', 'v2.non_bot_visits');
->from('(' . $tagsSubQb->getSQL() . ')', 't')
->leftJoin('t', '(' . $allVisitsSubQb->getSQL() . ')', 'v', $mainQb->expr()->eq('t.tag_id', 'v.tag_id'))
->leftJoin('t', '(' . $nonBotVisitsSubQb->getSQL() . ')', 'b', $mainQb->expr()->eq('t.tag_id', 'b.tag_id'))
->setMaxResults($filtering?->limit ?? PHP_INT_MAX)
->setFirstResult($filtering?->offset ?? 0);
// Apply API key role conditions to the native query too, as they will affect the amounts on the aggregates
$applyApiKeyToNativeQuery($apiKey, $nativeQb);
if ($orderMainQuery) {
$nativeQb
->orderBy(OrderableField::toSnakeCaseValidField($orderField), $orderDir ?? 'ASC')
->setMaxResults($filtering?->limit ?? PHP_INT_MAX)
->setFirstResult($filtering?->offset ?? 0);
$mainQb->orderBy(camelCaseToSnakeCase($orderField->value), $orderDir);
if ($orderField !== OrderableField::TAG) {
// Add ordering by tag name, as a fallback in case of same amounts
$mainQb->addOrderBy('tag', 'ASC');
}
// Add ordering by tag name, as a fallback in case of same amount, or as default ordering
$nativeQb->addOrderBy('t.name_1', $orderMainQuery || $orderDir === null ? 'ASC' : $orderDir);
$rsm = new ResultSetMappingBuilder($this->getEntityManager());
$rsm->addScalarResult('name', 'tag');
$rsm->addScalarResult('tag', 'tag');
$rsm->addScalarResult('visits', 'visits');
$rsm->addScalarResult('non_bot_visits', 'nonBotVisits');
$rsm->addScalarResult('short_urls_count', 'shortUrlsCount');
return map(
$this->getEntityManager()->createNativeQuery($nativeQb->getSQL(), $rsm)->getResult(),
$this->getEntityManager()->createNativeQuery($mainQb->getSQL(), $rsm)->getResult(),
TagInfo::fromRawData(...),
);
}

4
module/Core/src/Tag/TagService.php
View File

@ -59,7 +59,7 @@ class TagService implements TagServiceInterface
*/
public function deleteTags(array $tagNames, ?ApiKey $apiKey = null): void
{
if ($apiKey !== null && ! $apiKey->isAdmin()) {
if (! ApiKey::isAdmin($apiKey)) {
throw ForbiddenTagOperationException::forDeletion();
}
@ -75,7 +75,7 @@ class TagService implements TagServiceInterface
*/
public function renameTag(TagRenaming $renaming, ?ApiKey $apiKey = null): Tag
{
if ($apiKey !== null && ! $apiKey->isAdmin()) {
if (! ApiKey::isAdmin($apiKey)) {
throw ForbiddenTagOperationException::forRenaming();
}

2
module/Core/test-db/ShortUrl/Repository/ShortUrlRepositoryTest.php
View File

@ -395,7 +395,7 @@ class ShortUrlRepositoryTest extends DatabaseTestCase
#[Test]
public function importedShortUrlsAreFoundWhenExpected(): void
{
$buildImported = static fn (string $shortCode, ?String $domain = null) =>
$buildImported = static fn (string $shortCode, ?string $domain = null) =>
new ImportedShlinkUrl(ImportSource::BITLY, 'foo', [], Chronos::now(), $domain, $shortCode, null);
$shortUrlWithoutDomain = ShortUrl::fromImport($buildImported('my-cool-slug'), true);

4
module/Rest/src/ApiKey/Spec/WithApiKeySpecsEnsuringJoin.php
View File

@ -11,14 +11,14 @@ use Shlinkio\Shlink\Rest\Entity\ApiKey;
class WithApiKeySpecsEnsuringJoin extends BaseSpecification
{
public function __construct(private ?ApiKey $apiKey, private string $fieldToJoin = 'shortUrls')
public function __construct(private readonly ?ApiKey $apiKey, private readonly string $fieldToJoin = 'shortUrls')
{
parent::__construct();
}
protected function getSpec(): Specification
{
return $this->apiKey === null || $this->apiKey->isAdmin() ? Spec::andX() : Spec::andX(
return $this->apiKey === null || ApiKey::isAdmin($this->apiKey) ? Spec::andX() : Spec::andX(
Spec::join($this->fieldToJoin, 's'),
$this->apiKey->spec($this->fieldToJoin),
);

26
module/Rest/src/ApiKey/Spec/WithInlinedApiKeySpecsEnsuringJoin.php
View File

@ -1,26 +0,0 @@
<?php
declare(strict_types=1);
namespace Shlinkio\Shlink\Rest\ApiKey\Spec;
use Happyr\DoctrineSpecification\Spec;
use Happyr\DoctrineSpecification\Specification\BaseSpecification;
use Happyr\DoctrineSpecification\Specification\Specification;
use Shlinkio\Shlink\Rest\Entity\ApiKey;
class WithInlinedApiKeySpecsEnsuringJoin extends BaseSpecification
{
public function __construct(private readonly ?ApiKey $apiKey, private readonly string $fieldToJoin = 'shortUrls')
{
parent::__construct();
}
protected function getSpec(): Specification
{
return $this->apiKey === null || $this->apiKey->isAdmin() ? Spec::andX() : Spec::andX(
Spec::join($this->fieldToJoin, 's'),
$this->apiKey->inlinedSpec(),
);
}
}

7
module/Rest/src/Entity/ApiKey.php
View File

@ -114,9 +114,12 @@ class ApiKey extends AbstractEntity
return Spec::andX(...$specs);
}
public function isAdmin(): bool
/**
* @return ($apiKey is null ? true : boolean)
*/
public static function isAdmin(?ApiKey $apiKey): bool
{
return $this->roles->isEmpty();
return $apiKey === null || $apiKey->roles->isEmpty();
}
public function hasRole(Role $role): bool